Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Some flaky test cases fail and pass after retry #170

Closed
dai-chen opened this issue Aug 28, 2019 · 4 comments
Closed

Some flaky test cases fail and pass after retry #170

dai-chen opened this issue Aug 28, 2019 · 4 comments
Labels
maintenance Improves code quality, but not the product

Comments

@dai-chen
Copy link
Member

We should remove any undetermined factor in test code, such as time, probability, float precision etc. Here is one example:

com.amazon.opendistroforelasticsearch.sql.unittest.metrics.RollingCounterTest > add FAILED
    java.lang.AssertionError:
    Expected: <6L>
         but: was <0L>
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:18)
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:6)
        at com.amazon.opendistroforelasticsearch.sql.unittest.metrics.RollingCounterTest.add(RollingCounterTest.java:59)


Suite: Test class com.amazon.opendistroforelasticsearch.sql.unittest.metrics.RollingCounterTest
  2> java.lang.AssertionError:
    Expected: <6L>
         but: was <0L>
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:18)
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:6)
        at com.amazon.opendistroforelasticsearch.sql.unittest.metrics.RollingCounterTest.add(RollingCounterTest.java:59)
@dai-chen dai-chen added the maintenance Improves code quality, but not the product label Aug 28, 2019
@dai-chen
Copy link
Member Author

Another example:

  2> java.lang.AssertionError:
    Expected: <0>
         but: was <1>
        at __randomizedtesting.SeedInfo.seed([7A0996D9620F7648:2D7436F5238F60B8]:0)
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:18)
        at org.junit.Assert.assertThat(Assert.java:956)
        at org.junit.Assert.assertThat(Assert.java:923)
        at com.amazon.opendistroforelasticsearch.sql.esintgtest.JoinIT.hintLimits_firstLimitSecondLimitOnlyOne(JoinIT.java:837)
        at com.amazon.opendistroforelasticsearch.sql.esintgtest.JoinIT.hintLimits_firstLimitSecondLimitOnlyOneHASH(JoinIT.java:249)

@galkk
Copy link
Contributor

galkk commented Aug 29, 2019

This particular issue is fixed

@galkk galkk closed this as completed Aug 29, 2019
@dai-chen dai-chen changed the title Some test case fails but could pass the next run sometimes Some flaky test cases fail and pass after retry Sep 10, 2019
@dai-chen dai-chen reopened this Sep 10, 2019
@dai-chen
Copy link
Member Author

hintLimits_firstLimitSecondLimitOnlyOneHASH failed a lot recently. This test case is to test deprecated hint JOIN_TABLES_LIMIT which limit the #documents returned from each joined index. Because of the limit on unsorted #docs, the join result differs. I think we're safe to just remove this test case as the cost of rerunning all unit test and IT increases.

@dai-chen
Copy link
Member Author

Fixed easily broken tests found so far. Closing for now. Will reopen if any more changes needed in future.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
maintenance Improves code quality, but not the product
Projects
None yet
Development

No branches or pull requests

2 participants