Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Tests] AbstractBuilderTestCase reproducibility is broken #32400

Closed
cbuescher opened this issue Jul 26, 2018 · 1 comment
Closed

[Tests] AbstractBuilderTestCase reproducibility is broken #32400

cbuescher opened this issue Jul 26, 2018 · 1 comment
Assignees
@cbuescher
Labels
:Search/Search Search-related issues that do not fall into other categories >test Issues or PRs that are addressing/adding tests v7.0.0-beta1

Comments

@cbuescher
Copy link
Member

cbuescher commented Jul 26, 2018 •
edited
Loading

Rest extending AbstractBuilderTestCase (essential all Query- and AggregationBuilder unit test) are not reproducible with the info that we log on test failure. The random values generated in each test method change depending on whether the method is run separately (e.g. with -Dtest.method) or the whole test suite is run. This happens even when fixing the full seed and not just the master seed.

To demonstrate this, add the following test method to e.g. RangeQueryBuilderTests:

    public void testRandomness() {
        assertEquals(-7481642973173394925L, randomLong());
    }

The whole test suite passes with 97B45898EB8C54FC as the master seed:

./gradlew :server:test -Dtests.seed=97B45898EB8C54FC -Dtests.class=org.elasticsearch.index.query.RangeQueryBuilderTests

However, only running with -Dtests.method="testRandomness", this fails with a different random long value than expected:

./gradlew :server:test -Dtests.seed=97B45898EB8C54FC -Dtests.class=org.elasticsearch.index.query.RangeQueryBuilderTests -Dtests.method="testRandomness"

   > Throwable #1: java.lang.AssertionError: expected:<-7481642973173394925> but was:<-6611281964877420649>
   >    at __randomizedtesting.SeedInfo.seed([97B45898EB8C54FC:F0156D024791DA56]:0)

Using the full seed as reported above when running the full suite works, so this passes:

./gradlew :server:test -Dtests.seed=97B45898EB8C54FC:F0156D024791DA56  -Dtests.class=org.elasticsearch.index.query.RangeQueryBuilderTests

But fails when using the REPRODUCE log line that includes the "-Dtests.method" parameter:

./gradlew :server:test -Dtests.seed=97B45898EB8C54FC:F0156D024791DA56 \
 -Dtests.class=org.elasticsearch.index.query.RangeQueryBuilderTests -Dtests.method="testRandomness"
@cbuescher cbuescher added >test Issues or PRs that are addressing/adding tests :Search/Search Search-related issues that do not fall into other categories labels Jul 26, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

@cbuescher cbuescher self-assigned this Jul 26, 2018
@cbuescher cbuescher mentioned this issue Jul 26, 2018
Closed
cbuescher pushed a commit to cbuescher/elasticsearch that referenced this issue Jul 26, 2018
Fix test reproducability in AbstractBuilderTestCase setup
2645f3f 
Currently AbstractBuilderTestCase generates certain random values in its
`beforeTest()` method annotated with @before only the first time that a test
method in the suite is run. This changes the values of subsequent random values
and has the effect that when running single methods from a test suite with
"-Dtests.method=*", the random values it sees are different from when the same
test method is run as part of the whole test suite. This makes it harder to use
the reproduction lines logged on failure.

This changes the above method so that `beforeTest` will do the same call to the
random value source each time a test method is run, so reproduction by running
just one method is possible again.

Closes elastic#32400
@cbuescher cbuescher mentioned this issue Jul 26, 2018
Merged
cbuescher pushed a commit that referenced this issue Aug 10, 2018
Fix test reproducability in AbstractBuilderTestCase setup (#32403)
22f7b03 
Currently AbstractBuilderTestCase generates certain random values in its
`beforeTest()` method annotated with @before only the first time that a test
method in the suite is run while initializing the serviceHolder that we use for
the rest of the test. This changes the values of subsequent random values
and has the effect that when running single methods from a test suite with
"-Dtests.method=*", the random values it sees are different from when the same
test method is run as part of the whole test suite. This makes it hard to use
the reproduction lines logged on failure.

This change runs the inialization of the serviceHolder and the randomization 
connected to it using the test runners master seed, so reproduction by running
just one method is possible again.


Closes #32400
cbuescher pushed a commit that referenced this issue Aug 10, 2018
Fix test reproducability in AbstractBuilderTestCase setup (#32403)
76911b8 
Currently AbstractBuilderTestCase generates certain random values in its
`beforeTest()` method annotated with @before only the first time that a test
method in the suite is run while initializing the serviceHolder that we use for
the rest of the test. This changes the values of subsequent random values
and has the effect that when running single methods from a test suite with
"-Dtests.method=*", the random values it sees are different from when the same
test method is run as part of the whole test suite. This makes it hard to use
the reproduction lines logged on failure.

This change runs the inialization of the serviceHolder and the randomization
connected to it using the test runners master seed, so reproduction by running
just one method is possible again.

Closes #32400
cbuescher pushed a commit that referenced this issue Aug 10, 2018
Fix test reproducability in AbstractBuilderTestCase setup (#32403)
5bb20f5 
Currently AbstractBuilderTestCase generates certain random values in its
`beforeTest()` method annotated with @before only the first time that a test
method in the suite is run while initializing the serviceHolder that we use for
the rest of the test. This changes the values of subsequent random values
and has the effect that when running single methods from a test suite with
"-Dtests.method=*", the random values it sees are different from when the same
test method is run as part of the whole test suite. This makes it hard to use
the reproduction lines logged on failure.

This change runs the inialization of the serviceHolder and the randomization
connected to it using the test runners master seed, so reproduction by running
just one method is possible again.

Closes #32400
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@cbuescher cbuescher

Labels
:Search/Search Search-related issues that do not fall into other categories >test Issues or PRs that are addressing/adding tests v7.0.0-beta1
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@colings86 @cbuescher @elasticmachine