Re: Avoiding Unpredictability in Our DUnit Testing Rules
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 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 > 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
Re: Avoiding Unpredictability in Our DUnit Testing Rules
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 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 >
Re: Avoiding Unpredictability in Our DUnit Testing Rules
Thanks for the correction Bruce! > On Nov 26, 2018, at 10:04 AM, Bruce Schuchardt wrote: > > While it's common to use VM.getVM(int) to get a VM you still must use > Host.getHost() to get a VM running an older version of Geode. Until someone > adds a VM.getVM(version, vmNumber) we can't frown upon > Host.getHost(0).getVM(startingVersion, 0). > > On 11/21/18 4:09 PM, Mark Hanson 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 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 wrote: Great find, Patrick. I hope this shakes out some of the test bugs! On Wed, Nov 21, 2018, 10:34 Patrick Rhomberg >>> > 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 >
Re: Avoiding Unpredictability in Our DUnit Testing Rules
While it's common to use VM.getVM(int) to get a VM you still must use Host.getHost() to get a VM running an older version of Geode. Until someone adds a VM.getVM(version, vmNumber) we can't frown upon Host.getHost(0).getVM(startingVersion, 0). On 11/21/18 4:09 PM, Mark Hanson 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 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 wrote: Great find, Patrick. I hope this shakes out some of the test bugs! On Wed, Nov 21, 2018, 10:34 Patrick Rhomberg 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
Re: Avoiding Unpredictability in Our DUnit Testing Rules
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 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 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 > wrote: > >> > >> Great find, Patrick. I hope this shakes out some of the test bugs! > >> > >> On Wed, Nov 21, 2018, 10:34 Patrick Rhomberg 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 > >>> > > > >
Re: Avoiding Unpredictability in Our DUnit Testing Rules
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 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 wrote: >> >> Great find, Patrick. I hope this shakes out some of the test bugs! >> >> On Wed, Nov 21, 2018, 10:34 Patrick Rhomberg > >>> 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 >>> >
Re: Avoiding Unpredictability in Our DUnit Testing Rules
Within a test itself, reaching out to the host / VM that way should be fine, since you know that it will be executing after any rule's before() method. On Wed, Nov 21, 2018 at 10:49 AM Nabarun Nag 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 > wrote: > > > > Great find, Patrick. I hope this shakes out some of the test bugs! > > > > On Wed, Nov 21, 2018, 10:34 Patrick Rhomberg 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 > >> > >
Re: Avoiding Unpredictability in Our DUnit Testing Rules
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 wrote: > > Great find, Patrick. I hope this shakes out some of the test bugs! > > On Wed, Nov 21, 2018, 10:34 Patrick Rhomberg >> 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 >>
Re: Avoiding Unpredictability in Our DUnit Testing Rules
Great find, Patrick. I hope this shakes out some of the test bugs! On Wed, Nov 21, 2018, 10:34 Patrick Rhomberg 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 >
Avoiding Unpredictability in Our DUnit Testing Rules
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
