Typo fix: If you need to use CleanupDUnitVMsRule along with other dunit rules, then you will need to* use RuleChain*.
If you don't need to use CleanupDUnitVMsRule then you don't need to use RuleChain. On Mon, Nov 26, 2018 at 11:57 AM Kirk Lund <kl...@apache.org> wrote: > Actually the only problem with this is specific to bouncing of dunit VMs. > I filed "GEODE-6033: DistributedTest rules should support VM bounce" > earlier this month and I have a branch with preliminary changes that seem > to be working fine. > > Aside from bouncing of dunit VMs, the dunit rules you listed do not care > about ordering of invocation, so you do *not* need to use RuleChain > (unless you're using CleanupDUnitVMsRule). There are two caveats though: > > 1) if you want or need to specify the number of VMs to be something other > than the default of 4, then you'll need to specify the number of VMs for > every one of these dunit rules (see example below) > > 2) these dunit rules do not currently work with bouncing of vms > > If you need to use CleanupDUnitVMsRule along with other dunit rules, then > you will need to use. If you need to use VM bouncing within a test, then > you'll need to wait until I can merge my > (I have a branch on which I've made some changes to make this work) > > So it's actually #2 that's causing your problem. But as I mentioned, I do > have a branch on which it does now work with bouncing of VMs but I'm not > quite ready to merge it. > > Here's an example for a test that wants to limit the number of dunit VMs > to 2. RuleChain is not needed, but you do have to specify the # of dunit > VMs on all 3 dunit rules: > > @Rule > public DistributedRule distributedRule = new DistributedRule(2); > > @Rule > public SharedCountersRule sharedCountersRule = new SharedCountersRule(2); > > @Rule > public SharedErrorCollector errorCollector = new SharedErrorCollector(2); > > Please don't use ManagementTestRule at all. I'm trying to modify all tests > that use it to use DistributedRule with Geode User APIs so that I can > delete it. > > All of those other dunit rules in your list extend AbstractDistributedRule > which is why rule ordering does not matter (unless you use > CleanupDUnitVMsRule because it bounces VMs). > > Thanks, > Kirk > > On Wed, Nov 21, 2018 at 10:34 AM 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 >> >
