You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2022/11/03 14:41:01 UTC
[GitHub] [ignite-3] MohsenDehghankar opened a new pull request, #1312: Fix flaky test failure in ConfigurationAsmGeneratorTest#testPolymorphicErrors
MohsenDehghankar opened a new pull request, #1312:
URL: https://github.com/apache/ignite-3/pull/1312
### What is the purpose of this PR
- This PR fixes the flaky test `org.apache.ignite.internal.configuration.asm.ConfigurationAsmGeneratorTest.testPolymorphicErrors`
- The mentioned test may fail or pass without changes made to the source code when it is run in different JVMs due to `Class#getDeclaredFields`'s non-deterministic output order.
### Why the test fails
- According to [Java documentation](https://docs.oracle.com/javase/8/docs/api/java/lang/Class.html#getDeclaredFields--) about `getDeclaredFields`, returned array are not sorted and are not in any particular order.
- `ConfigurationUtil#schemaFields()` uses `getDeclaredFields` to return the list of schema fields. This list is passed to `ConfigurationAsmGenerator#createNodeClass()` and then passed to `ConfigurationAsmGenerator#addNodeTraverseChildrenMethod()`. In this method at line 1249, there is an assertion to check if `schemaFields.get(0)` is PolymorphicId, but due to the non-deterministic order of this list, the first index of the list might not be PolymorphicId and assertion fails.
### Reproduce the test failure
- Run the test with 'NonDex' maven plugin. The command to recreate the flaky test failure is `mvn edu.illinois:nondex-maven-plugin:2.1-SNAPSHOT:nondex -pl modules/configuration -Dtest=ConfigurationAsmGeneratorTest#testPolymorphicErrors`
- Fixing the flaky test now, will prevent flaky test failures in future Java versions.
### Expected Result
- The tests should run successfully when run with 'NonDex' maven with the same command for recreating the flaky test.
### Actual Result
- We get the following error:
```
[INFO] Running org.apache.ignite.internal.configuration.asm.ConfigurationAsmGeneratorTest
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.107 s <<< FAILURE! - in org.apache.ignite.internal.configuration.asm.ConfigurationAsmGeneratorTest
[ERROR] org.apache.ignite.internal.configuration.asm.ConfigurationAsmGeneratorTest.testPolymorphicErrors Time elapsed: 0.085 s <<< FAILURE!
java.lang.AssertionError: org.apache.ignite.internal.configuration.asm.ConfigurationAsmGeneratorTest$PolymorphicNamedTestConfigurationSchema
at org.apache.ignite.internal.configuration.asm.ConfigurationAsmGeneratorTest.beforeEach(ConfigurationAsmGeneratorTest.java:110)
```
### Fix
- Instead of checking `ConfigurationUtil#isPolymorphicId` on the first element of `schemaFields`, use `schemaFields.stream().anyMatch` to check the whole list.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] MohsenDehghankar commented on a diff in pull request #1312: IGNITE-18080 Fix flaky test failure in ConfigurationAsmGeneratorTest#testPolymorphicErrors
Posted by GitBox <gi...@apache.org>.
MohsenDehghankar commented on code in PR #1312:
URL: https://github.com/apache/ignite-3/pull/1312#discussion_r1013757747
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/ConfigurationAsmGenerator.java:
##########
@@ -1246,7 +1246,7 @@ private static void addNodeTraverseChildrenMethod(
);
} else if (!polymorphicFieldsByExtension.isEmpty()) {
assert polymorphicTypeIdFieldDef != null : schemaClass.getName();
- assert isPolymorphicId(schemaFields.get(0)) : schemaClass.getName();
+ assert schemaFields.stream().anyMatch(ConfigurationUtil::isPolymorphicId) : schemaClass.getName();
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #1312: IGNITE-18080 Fix flaky test failure in ConfigurationAsmGeneratorTest#testPolymorphicErrors
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #1312:
URL: https://github.com/apache/ignite-3/pull/1312#discussion_r1013687800
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/ConfigurationAsmGenerator.java:
##########
@@ -1246,7 +1246,7 @@ private static void addNodeTraverseChildrenMethod(
);
} else if (!polymorphicFieldsByExtension.isEmpty()) {
assert polymorphicTypeIdFieldDef != null : schemaClass.getName();
- assert isPolymorphicId(schemaFields.get(0)) : schemaClass.getName();
+ assert schemaFields.stream().anyMatch(ConfigurationUtil::isPolymorphicId) : schemaClass.getName();
Review Comment:
Please modify the documentation for `org.apache.ignite.configuration.annotation.PolymorphicConfig`, `org.apache.ignite.configuration.annotation.PolymorphicId` and `modules/configuration/README.md`.
And also please add text to the assertion, for example: `"Missing field with @PolymorphicId in " + schemaClass.getName();`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #1312: IGNITE-18080 Fix flaky test failure in ConfigurationAsmGeneratorTest#testPolymorphicErrors
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on code in PR #1312:
URL: https://github.com/apache/ignite-3/pull/1312#discussion_r1013687800
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/ConfigurationAsmGenerator.java:
##########
@@ -1246,7 +1246,7 @@ private static void addNodeTraverseChildrenMethod(
);
} else if (!polymorphicFieldsByExtension.isEmpty()) {
assert polymorphicTypeIdFieldDef != null : schemaClass.getName();
- assert isPolymorphicId(schemaFields.get(0)) : schemaClass.getName();
+ assert schemaFields.stream().anyMatch(ConfigurationUtil::isPolymorphicId) : schemaClass.getName();
Review Comment:
Please modify the documentation for `org.apache.ignite.configuration.annotation.PolymorphicConfig`.
And also please add text to the assertion, for example: `"Missing field with @PolymorphicId in " + schemaClass.getName();`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] tkalkirill merged pull request #1312: IGNITE-18080 Fix flaky test failure in ConfigurationAsmGeneratorTest#testPolymorphicErrors
Posted by GitBox <gi...@apache.org>.
tkalkirill merged PR #1312:
URL: https://github.com/apache/ignite-3/pull/1312
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org