Conversation
|
Simple fix and looks good to me. But what's confusing is that if the original tests is using arrays, are the orders of the arrays also a element to test? |
harshith2000
left a comment
There was a problem hiding this comment.
Hi, The fix looks good to me. But can you please confirm if these changes do not break the other tests in this module? You can check this by doing a test on the entire module.
Owner
Author
|
Thank you for the feedback. These changes do not break other tests. |
|
https://campuswire.com/c/G7A0E96FD/feed/501 good to go |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Created this PR to fix 6 flaky tests namely
AnnotationUtilTest.getAnnotationSyncAlias,AnnotationUtilTest.getAnnotationValueTest2,AnnotationUtilTest.getAnnotationValueTest,AnnotationUtilTest.getCombinationAnnotationsTest,AnnotationUtilTest.getCombinationAnnotationsWithClassTestandAnnotationUtilTest.scanMetaAnnotationTestwhich can be found here.
How was this test identified as flaky?
This test was identifies as flaky by using an open-source research tool named NonDex which is responsible for finding and diagnosing non-deterministic runtime exceptions in Java programs.
What do the tests do?
getAnnotationSyncAliasThis test verifies the functionality of retrieving and adapting annotations in Java, including handling aliases and ensuring synthesized annotation properties.
getAnnotationValueTest2This test validates the retrieval of annotation values from the
ClassWithAnnotationclass, specifically thenamesattribute ofAnnotationForTest, ensuring it matches an expected array.getAnnotationValueTestThis test checks if the value retrieved from the annotation
AnnotationForTeston theClassWithAnnotationclass is equal to the expected string.getCombinationAnnotationsTestThis test checks if the
getAnnotations()method of AnnotationUtil returns the expected array of annotations for a given class.getCombinationAnnotationsWithClassTestThis test verifies if the
getCombinationAnnotations()method of AnnotationUtil correctly retrieves annotations of a specific type for a given class.scanMetaAnnotationTestThis test scans for meta-annotations (annotations on other annotations) starting from the RootAnnotation class.
The issue with the first 5 tests is that we are comparing the equality of
declaredAnnotationsandannotationswithout paying attention to the order of the annotations. If we compare them without considering the order of the elements then theannotationMapcould sometimes be initialized to an empty TableMap which is incorrect. The following line is where the mentioned comparison happens.hutool/hutool-core/src/main/java/cn/hutool/core/annotation/CombinationAnnotationElement.java
Line 105 in 20f0c72
Because of this underlying issue, each of the 5 tests fail in the following lines:
getAnnotationSyncAliashutool/hutool-core/src/test/java/cn/hutool/core/annotation/AnnotationUtilTest.java
Line 55 in 20f0c72
getAnnotationValueTest2hutool/hutool-core/src/test/java/cn/hutool/core/annotation/AnnotationUtilTest.java
Line 45 in 20f0c72
getAnnotationValueTesthutool/hutool-core/src/test/java/cn/hutool/core/annotation/AnnotationUtilTest.java
Line 38 in 20f0c72
getCombinationAnnotationsTesthutool/hutool-core/src/test/java/cn/hutool/core/annotation/AnnotationUtilTest.java
Line 24 in 20f0c72
getCombinationAnnotationsWithClassTesthutool/hutool-core/src/test/java/cn/hutool/core/annotation/AnnotationUtilTest.java
Line 32 in 20f0c72
The issue with the 6th test is that the
getAnnotationsFromTargetClass()method returns thetargetAnnotationsin reverse order. This causes the order of annotations being compared in the test to be incorrect, causing the assertion to fail.hutool/hutool-core/src/test/java/cn/hutool/core/annotation/AnnotationUtilTest.java
Line 81 in 20f0c72
This PR fixes the first 5 tests by sorting both
declaredAnnotationsandannotationsbefore comparing their equality. It also fixes the 6th test by sorting the annotations in reverse order.You can run the following commands to run the tests using the NonDex tool:
(Optional) You can also run the following commands to run the test:
Test Environment:
Kindly let me know if these fixes are acceptable and if not, I would appreciate it if you could provide feedback/suggestions on my fixes. Thank you.