geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kirk Lund <kl...@apache.org>
Subject Re: Avoiding Unpredictability in Our DUnit Testing Rules
Date Mon, 26 Nov 2018 19:59:07 GMT
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 <klund@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 <prhomberg@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
>>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message