Is there a preferred ordering of the above list of rules? Or is the preferred ordering whichever passes?
On Wed, Nov 21, 2018 at 4:36 PM Mark Hanson <mhan...@pivotal.io> wrote: > It is frowned upon. VM.getVM(0) is now the accepted way to get VM 0. > > Thanks, > Mark > > > > On Nov 21, 2018, at 10:41 AM, Nabarun Nag <n...@pivotal.io> wrote: > > > > Will doing this in the test, > > > > final Host host = Host.getHost(0); > > VM server1 = host.getVM(startingVersion, 0); > > > > be frowned upon, if I use the above over using @Rule. > > > > Regards > > Nabarun Nag > > > >> On Nov 21, 2018, at 10:36 AM, Robert Houghton <rhough...@pivotal.io> > wrote: > >> > >> 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 > >>> > > > >
