QA: System property to override distribution #30591

nik9000 · 2018-05-14T19:25:38Z

This configures all qa projects to use the distribution contained in
the tests.distribution system property if it is set. The goal is to
create a simple way to run tests against the default distribution which
has x-pack basic features enabled while not forcing these tests on all
contributors. You run these tests by doing something like:

./gradlew -p qa -Dtests.distribution=zip check

or

./gradlew -p qa -Dtests.distribution=zip bwcTest

x-pack basic shouldn't get in the way of any of these tests but
nothing is ever perfect so this also adds support for skipping tests
based on the distribution. For the yaml tests this looks like:

    - skip:
        distribution: zip
        reason: x-pack includes templates which breaks the output here

For java tests it looks something like:

assumeFalse("zip".equals(System.getProperty("tests.distribution")),
        "some reason");

Gradle makes sure to set the tests.distribution environment variable
when running tests whether or not you override the distribution on the
command line. This allows us to remove some tests.rest.blacklist
settings in x-pack/qa because those tests skip themselves based on the
distribution. I think this sort of thing substantially reduces the
"surprise factor" while working on x-pack.

This configures all `qa` projects to use the distribution contained in the `tests.distribution` system property if it is set. The goal is to create a simple way to run tests against the default distribution which has x-pack basic features enabled while not forcing these tests on all contributors. You run these tests by doing something like: ``` ./gradlew -p qa -Dtests.distribution=zip check ``` or ``` ./gradlew -p qa -Dtests.distribution=zip bwcTest ``` x-pack basic *shouldn't* get in the way of any of these tests but nothing is ever perfect so this also adds support for skipping tests based on the distribution. For the yaml tests this looks like: ``` - skip: distribution: zip reason: x-pack includes templates which breaks the output here ``` For java tests it looks something like: ``` assumeFalse("zip".equals(System.getProperty("tests.distribution")), "some reason"); ``` Gradle makes sure to set the `tests.distribution` environment variable when running tests whether or not you override the distribution on the command line. This allows us to remove some `tests.rest.blacklist` settings in `x-pack/qa` because those tests skip themselves based on the distribution. I think this sort of thing substantially reduces the "surprise factor" while working on x-pack.

elasticmachine · 2018-05-14T19:25:40Z

Pinging @elastic/es-core-infra

nik9000 · 2018-05-14T19:26:08Z

qa/build.gradle

@@ -4,7 +4,7 @@ import org.elasticsearch.gradle.test.RestIntegTestTask
 subprojects { Project subproj ->
  subproj.tasks.withType(RestIntegTestTask) {
    subproj.extensions.configure("${it.name}Cluster") { cluster ->
-      cluster.distribution = 'oss-zip'
+      cluster.distribution = System.getProperty('tests.distribution', 'oss-zip')


At first I thought this PR would be just this line. If only things were that simple....

And now that I've reverted most of the rest, it is almost the only line!

nik9000 · 2018-05-14T19:26:54Z

test/framework/src/main/java/org/elasticsearch/test/junit/listeners/ReproduceInfoPrinter.java

@@ -149,6 +149,7 @@ public ReproduceErrorMessageBuilder appendESProperties() {
            }
            appendOpt("tests.locale", Locale.getDefault().toLanguageTag());
            appendOpt("tests.timezone", TimeZone.getDefault().getID());
+            appendOpt("tests.distribution", System.getProperty("tests.distribution"));


If you override the distribution on the command line then it is important to override it again. If you don't override it on the command line then setting this environment variable does nothing which is fine.

nik9000 · 2018-05-14T19:27:47Z

x-pack/qa/core-rest-tests-with-security/build.gradle

@@ -15,9 +15,6 @@ integTestRunner {
    ['cat.aliases/10_basic/Empty cluster',
     'index/10_with_id/Index with ID',
     'indices.get_alias/10_basic/Get alias against closed indices',
-     'cat.templates/10_basic/No templates',


These are skipped by the skip section in the test now which I find much easier to read.

nik9000 · 2018-05-14T19:28:15Z

This is a WIP because I'm still experimenting with it locally.

nik9000 · 2018-05-14T19:32:00Z

I think I could drop the skip section changes in favor of more blacklists but I prefer I think they make the tests more readable. If we feel like it is too big to backport to 6.3 then I'm happy to go the blacklist route and do the skip in a followup.

nik9000 · 2018-05-14T20:51:44Z

I think I could drop the skip section changes in favor of more blacklists but I prefer I think they make the tests more readable. If we feel like it is too big to backport to 6.3 then I'm happy to go the blacklist route and do the skip in a followup.

On second thought I think I will drop the skip enhancement. Those yml files are shared between Elasticsearch and its language clients are they aren't likely to like the change.

It'd be nice to have it but it'd break clients.

nik9000 · 2018-05-14T21:42:26Z

qa/mixed-cluster/build.gradle

@@ -57,6 +57,11 @@ for (Version version : bwcVersions.wireCompatible) {
  /* To support taking index snapshots, we have to set path.repo setting */
  tasks.getByName("${baseName}#mixedClusterTestRunner").configure {
    systemProperty 'tests.path.repo', new File(buildDir, "cluster/shared/repo")
+    systemProperty 'tests.rest.blacklist', [


No, no, this isn't right.

nik9000 · 2018-05-15T00:22:38Z

not quite...

nik9000 · 2018-05-15T13:30:03Z

I'm still working through getting a clean run of ./gradlew -p qa check -Dtests.distribution=zip. Most things are working but it not everything. So far the only failing things are to do with the tests, not Elasticsearch.

nik9000 · 2018-05-15T14:09:52Z

OK! I've got ./gradlew -p qa check -Dtests.distribution=zip passing locally. I'm running ./gradlew -p qa bwcTest -Dtests.distribution=zip to make sure that that passes as well but I suspect it will.

nik9000 · 2018-05-15T15:08:44Z

OK! I got a successful run of ./gradlew -p qa bwcTest -Dtests.distribution=zip!

rjernst

LGTM

rjernst · 2018-05-15T20:20:29Z

qa/smoke-test-rank-eval-with-mustache/build.gradle

@@ -26,3 +26,10 @@ dependencies {
    testCompile project(path: ':modules:lang-mustache', configuration: 'runtime')
 }

+/*
+ * One of the integration tests doesn't work with the zip distribution


Can you open an issue for this so it is tracked and link it in this code comment?

Absolutely.

This configures all `qa` projects to use the distribution contained in the `tests.distribution` system property if it is set. The goal is to create a simple way to run tests against the default distribution which has x-pack basic features enabled while not forcing these tests on all contributors. You run these tests by doing something like: ``` ./gradlew -p qa -Dtests.distribution=zip check ``` or ``` ./gradlew -p qa -Dtests.distribution=zip bwcTest ``` x-pack basic *shouldn't* get in the way of any of these tests but nothing is ever perfect so this we have to disable a few when running with the zip distribution.

bleskes · 2018-05-16T08:44:46Z

It looks like this was backported. I removed the label.

…bad-mkay * elastic/6.x: [ML] Wait for ML indices in rolling upgrade tests (elastic#30615) Watcher: Ensure secrets integration tests also run triggered watch (elastic#30478) Move allocation awareness attributes to list setting (elastic#30626) [Docs] Update code snippet in has-child-query.asciidoc (elastic#30510) Allow date math for naming newly-created snapshots (elastic#7939) (elastic#30479) Awaits fix a failing test Switch many QA projects to use new style requests (elastic#30574) QA: System property to override distribution (elastic#30591)

nik9000 · 2018-05-16T18:40:22Z

It looks like this was backported. I removed the label.

Thanks!

nik9000 added 3 commits May 14, 2018 15:02

Merge branch 'master' into qa_tests_distribution

db8c1ef

Update message

d2d384e

nik9000 added >test Issues or PRs that are addressing/adding tests blocker :Delivery/Build Build or test infrastructure v7.0.0 v6.3.0 v6.4.0 labels May 14, 2018

nik9000 commented May 14, 2018

View reviewed changes

nik9000 added 5 commits May 14, 2018 16:52

Drop skip change

5b64f0e

It'd be nice to have it but it'd break clients.

Drop skip changes

9cc15bd

revert more

39440af

Revert yet more

6d9e590

Revert remaining

f3ef5bf

nik9000 changed the title ~~WIP: QA: System property to override distribution~~ QA: System property to override distribution May 14, 2018

nik9000 commented May 14, 2018

View reviewed changes

Skip only if distribution is wrong

8465558

nik9000 added the review label May 14, 2018

nik9000 mentioned this pull request May 14, 2018

Test upgrading to the default distribution #30431

Closed

nik9000 removed the review label May 15, 2018

fix rank eval

62b0500

Add reproduce info

25c81a3

nik9000 added the review label May 15, 2018

rjernst approved these changes May 15, 2018

View reviewed changes

nik9000 mentioned this pull request May 15, 2018

smoke-test-rank-eval-with-mustache doesn't work with zip distribution #30628

Closed

track

d38e00a

nik9000 merged commit 869b639 into elastic:master May 15, 2018

nik9000 added the backport pending label May 15, 2018

bleskes removed the backport pending label May 16, 2018

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

QA: System property to override distribution #30591

QA: System property to override distribution #30591

nik9000 commented May 14, 2018

elasticmachine commented May 14, 2018

nik9000 May 14, 2018

nik9000 May 14, 2018

nik9000 May 14, 2018

nik9000 May 14, 2018

nik9000 commented May 14, 2018

nik9000 commented May 14, 2018

nik9000 commented May 14, 2018

nik9000 May 14, 2018

nik9000 commented May 15, 2018

nik9000 commented May 15, 2018

nik9000 commented May 15, 2018

nik9000 commented May 15, 2018

rjernst left a comment

rjernst May 15, 2018

nik9000 May 15, 2018

bleskes commented May 16, 2018

nik9000 commented May 16, 2018

QA: System property to override distribution #30591

QA: System property to override distribution #30591

Conversation

nik9000 commented May 14, 2018

elasticmachine commented May 14, 2018

nik9000 May 14, 2018

Choose a reason for hiding this comment

nik9000 May 14, 2018

Choose a reason for hiding this comment

nik9000 May 14, 2018

Choose a reason for hiding this comment

nik9000 May 14, 2018

Choose a reason for hiding this comment

nik9000 commented May 14, 2018

nik9000 commented May 14, 2018

nik9000 commented May 14, 2018

nik9000 May 14, 2018

Choose a reason for hiding this comment

nik9000 commented May 15, 2018

nik9000 commented May 15, 2018

nik9000 commented May 15, 2018

nik9000 commented May 15, 2018

rjernst left a comment

Choose a reason for hiding this comment

rjernst May 15, 2018

Choose a reason for hiding this comment

nik9000 May 15, 2018

Choose a reason for hiding this comment

bleskes commented May 16, 2018

nik9000 commented May 16, 2018