WFCORE-7376 eliminate testAddingNewResources non determinism #6533
Conversation
|
Hello, 18chanp1. I'm waiting for one of the admins to verify this patch with /ok-to-test in a comment. |
18chanp1
changed the title
| assertEquals(ADD, postExtensionOps.get(2).operationName); | ||
| assertEquals(PathAddress.pathAddress("property", "test-property"), postExtensionOps.get(2).address); | ||
| assertTrue(postExtensionOps.get(2).operation.hasDefined("props")); | ||
| ModelNode properties = postExtensionOps.get(2).operation.get("props").asObject(); | ||
| assertEquals("0", properties.get("ip_ttl").asString()); | ||
| assertEquals("5", properties.get("tcp_ttl").asString()); | ||
| assertEquals(ADD, postExtensionOps.get(0).operationName); | ||
| assertEquals(PathAddress.pathAddress("basic", "test"), postExtensionOps.get(0).address); | ||
| assertFalse(postExtensionOps.get(0).operation.hasDefined("value")); | ||
| assertEquals(ADD, postExtensionOps.get(1).operationName); | ||
| assertEquals(PathAddress.pathAddress("basic", "test"), postExtensionOps.get(1).address); | ||
| assertFalse(postExtensionOps.get(1).operation.hasDefined("value")); | ||
| assertEquals(ADD, postExtensionOps.get(2).operationName); | ||
| assertEquals(PathAddress.pathAddress("classpath", "runtime"), postExtensionOps.get(2).address); | ||
| ModelNode objectMap = postExtensionOps.get(2).operation.get("complex-map").asObject(); | ||
| assertEquals(PathAddress.pathAddress("classpath", "runtime"), postExtensionOps.get(1).address); | ||
| ModelNode objectMap = postExtensionOps.get(1).operation.get("complex-map").asObject(); |
There was a problem hiding this comment.
Also if you want, you can improve the readability of this test. For example, by sorting the code by the index of each postExtensionOps element and in groups separated by a blank line. I mean:
assertEquals(ADD, postExtensionOps.get(0).operationName);
assertEquals(PathAddress.pathAddress("basic", "test"), postExtensionOps.get(0).address);
assertFalse(postExtensionOps.get(0).operation.hasDefined("value"));
assertEquals(ADD, postExtensionOps.get(1).operationName);
assertEquals(PathAddress.pathAddress("classpath", "runtime"), postExtensionOps.get(1).address);
ModelNode objectMap = postExtensionOps.get(1).operation.get("complex-map").asObject();
... then the group that is asserting on postExtensionOps.get(2) ... and so on
|
Thanks for contributing to WildFly Core @18chanp1 ! This Pull Request (PR) and #6534 are targeting the same issue and the same testcase, so it is preferable a single PR instead of two. Could you add all your changes in a single PR instead? We do not need two commits, but that's up to you. A single PR with a single commit is even better here since we can later easily backport it to other branches when required. Also, notice that the commit message for each commit should also reference a JIRA number. |
18chanp1
force-pushed
the
fix7339
branch
2 times, most recently
from
6319a53 to
75757f3
Compare
|
Hi @yersan, thanks for the suggestions I've squashed the commits into 1 commit, combined it into this PR, added gaps. Please let me know if there are any additional changes to be made. Thanks |
yersan
left a comment
yersan
left a comment
There was a problem hiding this comment.
Thanks @18chanp1 , looks much better now. However, Job reported check style issues that need to be addressed before runnig the full testsuite to verify the testcase:
[INFO] --- checkstyle:3.6.0:checkstyle (check-style) @ wildfly-controller ---
[INFO] Rendering content with org.apache.maven.skins:maven-fluido-skin:jar:2.0.0-M9 skin
[INFO] Starting audit...
Error: /home/runner/work/wildfly-core/wildfly-core/pr/controller/src/test/java/org/jboss/as/controller/persistence/yaml/YamlConfigurationExtensionTest.java:219: Line has trailing spaces. [RegexpSingleline]
Error: /home/runner/work/wildfly-core/wildfly-core/pr/controller/src/test/java/org/jboss/as/controller/persistence/yaml/YamlConfigurationExtensionTest.java:231: Line has trailing spaces. [RegexpSingleline]
Error: /home/runner/work/wildfly-core/wildfly-core/pr/controller/src/test/java/org/jboss/as/controller/persistence/yaml/YamlConfigurationExtensionTest.java:243: Line has trailing spaces. [RegexpSingleline]
Error: /home/runner/work/wildfly-core/wildfly-core/pr/controller/src/test/java/org/jboss/as/controller/persistence/yaml/YamlConfigurationExtensionTest.java:248: Line has trailing spaces. [RegexpSingleline]
Error: /home/runner/work/wildfly-core/wildfly-core/pr/controller/src/test/java/org/jboss/as/controller/persistence/yaml/YamlConfigurationExtensionTest.java:253: Line has trailing spaces. [RegexpSingleline]
Audit done.
Hi @yersan, I've addressed the checkstyle issues |
yersan
left a comment
yersan
left a comment
There was a problem hiding this comment.
Thanks @18chanp1 , but they are still failing:
[INFO] Starting audit...
Error: /home/runner/work/wildfly-core/wildfly-core/pr/controller/src/test/java/org/jboss/as/controller/persistence/yaml/YamlConfigurationExtensionTest.java:253: Line has trailing spaces. [RegexpSingleline]
You can verify it locally by building the project with mvn clean install -DskipTests, ensure it can be built locally before pushing it up.
|
There has been no activity on this PR for 45 days. It will be auto-closed after 90 days. |
3 participants
https://issues.redhat.com/browse/WFCORE-7376
YamlConfigurationExtensionTest is indeterministic because there is no guarantee of the order of operations after processOperations is run.
The fix is to sort the list after running processOperations and then fixing the order instead.