commons-jxpath icon indicating copy to clipboard operation
commons-jxpath copied to clipboard
verified publisher icon apache

Fix Flaky Tests

Open qz0610 opened this issue 1 year ago • 3 comments

Description

Eleven tests were found to be flaky using nondex:

org.apache.commons.jxpath.ri.compiler.ExtensionFunctionTest#testExpressionContext
org.apache.commons.jxpath.ri.model.beans.BeanModelTest#testAxisDescendantOrSelf
org.apache.commons.jxpath.ri.model.beans.BeanModelTest#testAxisParent
org.apache.commons.jxpath.ri.model.beans.BeanModelTest#testAxisFollowing
org.apache.commons.jxpath.ri.model.beans.BeanModelTest#testBooleanPredicate
org.apache.commons.jxpath.ri.model.beans.BeanModelTest#testAxisDescendant
org.apache.commons.jxpath.ri.model.beans.BeanModelTest#testAxisFollowingSibling
org.apache.commons.jxpath.ri.model.dynabeans.DynaBeanModelTest#testAxisDescendantOrSelf
org.apache.commons.jxpath.ri.model.dynabeans.DynaBeanModelTest#testAxisParent
org.apache.commons.jxpath.ri.model.dynabeans.DynaBeanModelTest#testAxisFollowing
org.apache.commons.jxpath.ri.model.dynabeans.DynaBeanModelTest#testAxisDescendant

The flakiness can be reproduced by running:

mvn edu.illinois:nondex-maven-plugin:2.1.7:nondex \
-Dtest=[test_path] \
-Drat.skip=true

flaky tests failures:

[ERROR]   ExtensionFunctionTest.testExpressionContext:277->JXPathTestCase.assertXPathValue:49 Evaluating <test:count(//strings)> expected:<21> but was:<24>
[ERROR]   BeanModelTest>BeanModelTestCase.testAxisDescendant:388->JXPathTestCase.assertXPathValue:49 Evaluating <count(descendant::node())> expected:<65.0> but was:<59.0>
[ERROR]   BeanModelTest>BeanModelTestCase.testAxisDescendantOrSelf:411->JXPathTestCase.assertXPathValueIterator:152 Evaluating value iterator <//name> expected:<[Name 2, Name 0, Name 5, Name 6, Name 4, Name 3, Name 1]> but was:<[Name 1, Name 2, Name 3, Name 0, Name 5, Name 6]>
[ERROR]   BeanModelTest>BeanModelTestCase.testAxisFollowing:445->JXPathTestCase.assertXPathValue:49 Evaluating <count(nestedBean/strings[2]/following::node())> expected:<21.0> but was:<15.0>
[ERROR]   BeanModelTest>BeanModelTestCase.testAxisFollowingSibling:480->JXPathTestCase.assertXPathValue:49 Evaluating <count(/descendant::boolean/following-sibling::node())> expected:<53.0> but was:<63.0>
[ERROR]   BeanModelTest>BeanModelTestCase.testAxisParent:495->JXPathTestCase.assertXPathValue:49 Evaluating <count(//..)> expected:<9.0> but was:<11.0>
[ERROR]   BeanModelTest>BeanModelTestCase.testBooleanPredicate:681->JXPathTestCase.assertXPathValueIterator:152 Evaluating value iterator <//self::node()[name(.) = concat('n', 'a', 'm', 'e')]> expected:<[Name 1, Name 2, Name 3, Name 6, Name 0, Name 5, Name 4]> but was:<[Name 1, Name 2, Name 3, Name 6, Name 0, Name 5]>
[ERROR]   DynaBeanModelTest>BeanModelTestCase.testAxisDescendant:388->JXPathTestCase.assertXPathValue:49 Evaluating <count(descendant::node())> expected:<65.0> but was:<71.0>
[ERROR]   DynaBeanModelTest>BeanModelTestCase.testAxisDescendantOrSelf:436->JXPathTestCase.assertXPathValue:49 Evaluating <count(descendant-or-self::node())> expected:<66.0> but was:<60.0>
[ERROR]   DynaBeanModelTest>BeanModelTestCase.testAxisFollowing:450->JXPathTestCase.assertXPathValue:49 Evaluating <count(nestedBean/strings[2]/following::strings)> expected:<7.0> but was:<10.0>
[ERROR]   DynaBeanModelTest>BeanModelTestCase.testAxisParent:495->JXPathTestCase.assertXPathValue:49 Evaluating <count(//..)> expected:<9.0> but was:<10.0>

These eleven tests have the same root cause for flakiness: their passing depends on the order of elements stored in the set returned by getSet() in TestBean class, which is not deterministic with HashSet implementation.

Proposed Fix

This pr proposes a simple fix by replacing HashSet with LinkedHashSet in TestBean. After this fix, all eleven tests now pass consistently.

qz0610 avatar Nov 16 '24 10:11 qz0610

@qz0610

Please make your tool write its files to the target folder, NOT the root!

garydgregory avatar Nov 16 '24 15:11 garydgregory

Hello @qz0610

Please rebase on git master and explain how this failure went away:

[ERROR]   BeanModelTest>BeanModelTestCase.testBooleanPredicate:681->JXPathTestCase.assertXPathValueIterator:152 Evaluating value iterator <//self::node()[name(.) = concat('n', 'a', 'm', 'e')]> expected:<[Name 1, Name 2, Name 3, Name 6, Name 0, Name 5, Name 4]> but was:<[Name 1, Name 2, Name 3, Name 6, Name 0, Name 5]>

The change is the addition of a JUnit assertion.

Now, there is a new error, comparing ArrayList instances:

[ERROR]   BeanModelTest>AbstractBeanModelTest.testBooleanPredicate:681->AbstractJXPathTest.assertXPathValueIterator:151 [hashCode()] Evaluating value iterator <//self::node()[name(.) = concat('n', 'a', 'm', 'e')]>, expected.class class java.util.ArrayList(7): [Name 1, Name 2, Name 3, Name 6, Name 0, Name 5, Name 4], actual.class class java.util.ArrayList(8): [Name 1, Name 2, Name 3, Name 6, Name 0, Name 5, Name 4, Name 4] expected:<948329621> but was:<1659447818>

How is this possible when we get green builds?

  • Am I incorrectly reading these test results?
  • Are these bugs in the tool?

TY

garydgregory avatar Nov 16 '24 15:11 garydgregory

Hi @garydgregory, thank you very much for the review and feedback. For these two failures,

[ERROR]   BeanModelTest>BeanModelTestCase.testBooleanPredicate:681->JXPathTestCase.assertXPathValueIterator:152 Evaluating value iterator <//self::node()[name(.) = concat('n', 'a', 'm', 'e')]> expected:<[Name 1, Name 2, Name 3, Name 6, Name 0, Name 5, Name 4]> but was:<[Name 1, Name 2, Name 3, Name 6, Name 0, Name 5]>
[ERROR]   BeanModelTest>AbstractBeanModelTest.testBooleanPredicate:681->AbstractJXPathTest.assertXPathValueIterator:151 [hashCode()] Evaluating value iterator <//self::node()[name(.) = concat('n', 'a', 'm', 'e')]>, expected.class class java.util.ArrayList(7): [Name 1, Name 2, Name 3, Name 6, Name 0, Name 5, Name 4], actual.class class java.util.ArrayList(8): [Name 1, Name 2, Name 3, Name 6, Name 0, Name 5, Name 4, Name 4] expected:<948329621> but was:<1659447818>

the order of elements in Hashset is not always guaranteed, when each time it's passed to getValue(Object collection, final int index) in org.apache.commons.jxpath.util.ValueUtils; i.e. an element in Hashset doesn't have an absolute index. For example,

getValue(collection: {String 4, 4, Nested: Name 4}, index: 0) -> String 4
getValue(collection: {Nested: Name 4, String 4, 4}, index: 1) -> String 4
getValue(collection: {String 4, Nested: Name 4, 4}, index: 2) -> 4

In this case getValue() could not retrieve Nested: Name 4 correctly, and thus cause the above errors of missing element and changed hashcode in the resulting ArrayList. If we replace Hashset with LinkedHashset, Nested: Name 4 would always have index 2 and is guaranteed to be retrieved, therefore resolving the failures.

qz0610 avatar Dec 01 '24 01:12 qz0610