Great find, Patrick. I hope this shakes out some of the test bugs! On Wed, Nov 21, 2018, 10:34 Patrick Rhomberg <prhomb...@apache.org wrote:
> tl;dr: Use JUnit RuleChain. > ---- > > Hello all! > > Several [1] of our test @Rule classes make use of the fact that our DUnit > VMs Host is statically accessible to affect every test VM. For instance, > the SharedCountersRule initializes a counter in every VM, and the > CleanupDUnitVMsRule bounces VMs before and after each test. > > Problematically, JUnit rules applied in an unpredictable / JVM-dependent > ordering. [2] As a result, some flakiness we find in our tests may be the > result of unexpected interaction and ordering of our test rules. [3] > > Fortunately, a solution to this problem already exists. Rule ordering can > be imposed by JUnit's RuleChain. [4] > > In early exploration with this rule, some tests failed due to the RuleChain > not being serializable. However, as it should only apply to the test VM, > and given that it can be composed of (unannotated) rules that remain > accessible and serializable, it should be a simple matter of declaring the > offending field transient, as it will only be necessary in the test VM. > > So, you dear reader: while you're out there making Geode the best it can > be, if you find yourself in a test class that uses more than one rule > listed in [1], or if you notice some other rule not listed below that > reaches out to VMs as part of its @Before or @After, please update that > test to use the RuleChain to apply the rules in a predictable order. > > Imagination is Change. > ~Patrick > > [1] A probably-incomplete list of invasive rules can be found via > $> git grep -il inEveryVM | grep Rule.java > > geode-core/src/distributedTest/java/org/apache/geode/management/ManagementTestRule.java > geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/CacheRule.java > > geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/ClientCacheRule.java > > geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/DistributedDiskDirRule.java > > geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/DistributedRule.java > > geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/DistributedUseJacksonForJsonPathRule.java > > geode-dunit/src/main/java/org/apache/geode/test/dunit/rules/SharedCountersRule.java > > [2] See the documentation for rules here: > https://junit.org/junit4/javadoc/4.12/org/junit/Rule.html ; notably, > "However, > if there are multiple [Rule] fields (or methods) they will be applied in an > order that depends on your JVM's implementation of the reflection API, > which is undefined, in general." > > [3] For what it's worth, this was discovered after looking into why the > DistributedRule check fo suspicious strings caused the test *after* the > test that emitted the strings to fail. It's only tangentially related, but > got me looking into when and how the @After was getting applied. > > [4] https://junit.org/junit4/javadoc/4.12/org/junit/rules/RuleChain.html >
