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

Add Wildfly integration test #24147

Merged
merged 9 commits into from
Apr 21, 2017
Merged

Add Wildfly integration test #24147

merged 9 commits into from
Apr 21, 2017

Conversation

jasontedor
Copy link
Member

@jasontedor jasontedor commented Apr 18, 2017

An important use case for our users is deploying our clients inside of applications containers like Wildfly. Sometimes, we make changes that unintentionally break this use case. We need to know before we ship a release that we have broken such use cases. As Wildfly is one of the bigger application containers, this commit starts by adding an integration test that deploys an application using the transport client to Wildfly and ensures that all is well. Future work can add similar integration tests for the low-level and high-level REST clients.

Closes #24050

An important use case for our users is deploying our clients inside of
applications containers like Wildly. Sometimes, we make changes that
unintentionally break this use case. We need to know before we ship a
release that we have broken such use cases. As Wildfly is one of the
bigger application containers, this commit starts by adding an
integration test that deploys an application using the transport client
to Wildfly and ensures that all is well. Future work can add similar
integration tests for the low-level and high-level REST clients.
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.

Looks good, but I don't think we should be using elasticsearch.build in a qa test.


test.enabled = false

dependencyLicenses.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.

I don't think a qa test should have any of these tasks at all. Why isn't this using the standalone-rest-test plugin like other qa tests?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because it defines a test source set and so does war and thus there's a conflict. Maybe you can show me a better way out of that situation.

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 discussed this with @rjernst. We are going to create an examples directory and we can move the generation of the war to a subproject there. This test subproject can depend on that artifact. That will enable this subproject to depend apply standalone-rest-test. This will be done a follow-up.

@jasontedor
Copy link
Member Author

retest this please

task startWildfly {
dependsOn enableIPv6, writeElasticsearchProperties
doFirst {
final File script = new File(project.buildDir, "wildfly/wildfly.killer.sh")
Copy link
Member

Choose a reason for hiding this comment

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

oh my. Does this need a windows version?

Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't need one because we skip this test on Windows. I do not see a good reason to jump through a lot of hoops to try to make it work on Windows. Please feel free to disagree and let me know if you think otherwise.

Copy link
Member

Choose a reason for hiding this comment

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

Ah. Makes sense. It is probably worth leaving a comment here just in case.

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, I pushed 2c800ce for this.

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.

Ok, this LGTM. I have one suggestion which could be a followup. It would be cool to have a general set of examples for elasticsearch of this kind of thing. So if we moved the war build here into a wildfly example, the test could then just depend on the example project, and the test could then be standalone again.

* master: (61 commits)
  Build: Move plugin cli and tests to distribution tool (elastic#24220)
  Peer Recovery: remove maxUnsafeAutoIdTimestamp hand off (elastic#24243)
  Adds version 5.3.2 and backwards compatibility indices for 5.3.1
  Add utility method to parse named XContent objects with typed prefix (elastic#24240)
  MultiBucketsAggregation.Bucket should not extend Writeable (elastic#24216)
  Don't expose cleaned-up tasks as pending in PrioritizedEsThreadPoolExecutor (elastic#24237)
  Adds declareNamedObjects methods to ConstructingObjectParser (elastic#24219)
  ESIntegTestCase.indexRandom should not introduce types. (elastic#24202)
  Tests: Extend InternalStatsTests (elastic#24212)
  IndicesQueryCache should delegate the scorerSupplier method. (elastic#24209)
  Speed up parsing of large `terms` queries. (elastic#24210)
  [TEST] make sure that the random query_string query generator defines a default_field or a list of fields
  token_count type : add an option to count tokens (fix elastic#23227) (elastic#24175)
  Query string default field (elastic#24214)
  Make Aggregations an abstract class rather than an interface (elastic#24184)
  [TEST] ensure expected sequence no and version are set when index/delete engine operation has a document failure
  Extract batch executor out of cluster service (elastic#24102)
  Add 5.3.1 to bwc versions
  Added "release-state" support to plugin docs
  Added examples to cross cluster search of using cluster settings
  ...
@jasontedor jasontedor merged commit 2dd924b into elastic:master Apr 21, 2017
jasontedor added a commit that referenced this pull request Apr 21, 2017
An important use case for our users is deploying our clients inside of
applications containers like Wildly. Sometimes, we make changes that
unintentionally break this use case. We need to know before we ship a
release that we have broken such use cases. As Wildfly is one of the
bigger application containers, this commit starts by adding an
integration test that deploys an application using the transport client
to Wildfly and ensures that all is well. Future work can add similar
integration tests for the low-level and high-level REST clients.

Relates #24147
@jasontedor jasontedor deleted the wildfly branch April 21, 2017 18:41
asettouf pushed a commit to asettouf/elasticsearch that referenced this pull request Apr 23, 2017
An important use case for our users is deploying our clients inside of
applications containers like Wildly. Sometimes, we make changes that
unintentionally break this use case. We need to know before we ship a
release that we have broken such use cases. As Wildfly is one of the
bigger application containers, this commit starts by adding an
integration test that deploys an application using the transport client
to Wildfly and ensures that all is well. Future work can add similar
integration tests for the low-level and high-level REST clients.

Relates elastic#24147
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 v5.5.0 v6.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants