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.

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

Begin replacing static index tests with full restart tests #24846

Merged
merged 5 commits into from
May 26, 2017

Conversation

nik9000
Copy link
Member

@nik9000 nik9000 commented May 23, 2017

These tests spin up two nodes of an older version of Elasticsearch,
create some stuff, shut down the nodes, start the current version,
and verify that the created stuff works.

You can run gradle qa:full-cluster-restart:check to run these
tests against the head of the previous branch of Elasticsearch
(5.x for master, 5.4 for 5.x, etc) or you can run
gradle qa:full-cluster-restart:bwcTest to run this test against
all "index compatible" versions, one after the other. For master
this is every released version in the 5.x.y version and the tip
of the 5.x branch.

I'd love to add more to these tests in the future but these
currently just cover the functionality of the create_bwc_index.py
script and start to cover the assertions in the
OldIndexBackwardsCompatibilityIT test.

These tests spin up two nodes of an older version of Elasticsearch,
create some stuff, shut down the nodes, start the current version,
and verify that the created stuff works.

You can run `gradle qa:full-cluster-restart:check` to run these
tests against the head of the previous branch of Elasticsearch
(5.x for master, 5.4 for 5.x, etc) or you can run
`gradle qa:full-cluster-restart:bwcTest` to run this test against
all "index compatible" versions, one after the other. For master
this is every released version in the 5.x.y version *and* the tip
of the 5.x branch.

I'd love to add more to these tests in the future but these
currently just cover the functionality of the `create_bwc_index.py`
script and start to cover the assertions in the
`OldIndexBackwardsCompatibilityIT` test.
@nik9000 nik9000 added review >test Issues or PRs that are addressing/adding tests v6.0.0 labels May 23, 2017
@nik9000 nik9000 requested a review from rjernst May 23, 2017 15:51
Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this! This is a good first step. One thing I realized is to make this reproducible (not just this test, but any that use multiple rest tests which have randomization) we need to make sure the random seed passed is consistent. We can't rely on the generated random seed inside randomized runner, since each runner would get it's own unique independent seed.


configure(extensions.findByName("${baseName}#upgradedClusterTestCluster")) {
dependsOn oldClusterTestRunner,
"${baseName}#oldClusterTestCluster#node0.stop",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be able to depend on oldClusterTest instead of oldClusterTestRunner and the cluster will be shut down.

* after projects are evaluated. */
gradle.projectsEvaluated {
// Disable cleaning the repository so we can test loading a snapshot
tasks.getByName("${baseName}#upgradedClusterTestCluster#prepareCluster.cleanShared").enabled = false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is kind of sneaky...maybe have a flag on cluster configuration that means "don't create a shared clean task"? Something about "reuse"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll open a followup for this.

doc.field("int", randomInt(100));
doc.field("float", randomFloat());
// be sure to create a "proper" boolean (True, False) for the first document so that automapping is correct
doc.field("bool", i > 0 && supportsLenientBooleans ? randomLenientBoolean() : randomBoolean());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to test lenient booleans. Sure the old cluster supported them, but they supported proper booleans too. They are gone from master now, so let's just stick with keeping this simple, there is enough complicated stuff here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is important that we make sure we can query indices with old style booleans in there. I know that it'll work because we've already built the index, but I think it is worth checking.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had mistakenly thought master did not have any lenient booleans support. This sounds fine.

@nik9000
Copy link
Member Author

nik9000 commented May 26, 2017

One thing I realized is to make this reproducible (not just this test, but any that use multiple rest tests which have randomization) we need to make sure the random seed passed is consistent.

Yes.

@nik9000 nik9000 merged commit e072cc7 into elastic:master May 26, 2017
jasontedor added a commit to mashudong/elasticsearch that referenced this pull request May 28, 2017
* master: (38 commits)
  Fix Lucene version expectation
  Verify Lucene version constants
  Avoid double decrement on current query counter
  Remove the need for _UNRELEASED suffix in versions (elastic#24798)
  Adjust available and free bytes to be non-negative on huge FSes
  Begin replacing static index tests with full restart tests (elastic#24846)
  Fix plugin docs for using custom config dir
  Update context-suggest.asciidoc
  Move BWC version to 5.5 after backport
  Support Multiple Collapse Inner Hits
  Scripting: Rename CompiledType to FactoryType in ScriptContext (elastic#24897)
  Scripting: Make contexts available to ScriptEngine construction (elastic#24896)
  Mute index and relocate concurrently
  Build: Add back explicit exclusions and remove gradle exclusions (elastic#24879)
  Scripting: Move context definitions to instance type classes (elastic#24883)
  Build: Fix hadoop integ test error on windows (elastic#24885)
  Put mapping and index template requests do not need content type detection for 5.3.0+ (elastic#24835)
  Add the ability to store objects with a ScrollContext (elastic#24777)
  add docs example for Ingest scripts manipulating document metadata (elastic#24875)
  Fix error message if an incompatible node connects (elastic#24884)
  ...
nik9000 added a commit to nik9000/elasticsearch that referenced this pull request May 30, 2017
This adds an option to `ClusterConfiguration` to preserve the
`shared` directory when starting up a new cluster and switches
the `qa:full-cluster-restart` tests to use it rather than
disable the clean shared task.

Relates to elastic#24846
nik9000 added a commit that referenced this pull request Jun 5, 2017
This adds an option to `ClusterConfiguration` to preserve the
`shared` directory when starting up a new cluster and switches
the `qa:full-cluster-restart` tests to use it rather than
disable the clean shared task.

Relates to #24846
@nik9000 nik9000 deleted the full_restart_test branch June 7, 2017 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>test Issues or PRs that are addressing/adding tests v6.0.0-alpha2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants