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

Use fixture to test repository-url module #29355

Merged
merged 2 commits into from
Apr 4, 2018

Conversation

tlrx
Copy link
Member

@tlrx tlrx commented Apr 3, 2018

Pursuing the effort to test repository implementations using fixtures, this commit adds a YAML integration test for the repository-url module that uses a fixture to test URL based repositories on both http:// and file:// prefixes.

Before running the YAML integration tests, 3 differents repositories are created:

repository-fs
This repository of type fs is used to create the snapshots

repository-url
This repository of type url is used to test the snapshot restore over HTTP. It uses the urlFixture.

repository-file
This repository of type url is used to test the snapshot restore using a file://url.

@tlrx tlrx added >test Issues or PRs that are addressing/adding tests review :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v7.0.0 labels Apr 3, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@@ -1,3 +1,5 @@
import org.elasticsearch.gradle.test.AntFixture
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to have this under the license.

break;
}
} catch (Exception e) {
logger.debug("Failed to resolve inet address for allowed URL [{}], skipping", allowedUrl);
Copy link
Member

Choose a reason for hiding this comment

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

This seems like a pretty bad thing though. maybe we should throw it?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is required as the integration tests also declare http://snapshot.test* as an allowed url for some test, so I think we can keep it like this.

response = new Response(RestStatus.FORBIDDEN, emptyMap(), "text/plain", new byte[0]);
}
} else {
response = new Response(RestStatus.INTERNAL_SERVER_ERROR, emptyMap(), "text/plain", new byte[0]);
Copy link
Member

Choose a reason for hiding this comment

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

No "unsupported HTTP method" message? :)

Copy link
Member Author

Choose a reason for hiding this comment

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

That's because I'm lazy :) I added the message.

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.

This is great, thanks @tlrx!

tlrx added 2 commits April 4, 2018 10:13
Pursuing the effort to test repository implementations using fixtures,
this commit adds a YAML integration test for the repository-url module
that uses a fixture to test URL based repositories on both http:// and
file:// prefixes.
@tlrx tlrx force-pushed the add-fixture-for-repository-url branch from 89cbd6e to 9a0a65c Compare April 4, 2018 08:14
@tlrx tlrx merged commit 08abbdf into elastic:master Apr 4, 2018
@tlrx tlrx deleted the add-fixture-for-repository-url branch April 4, 2018 13:55
@tlrx
Copy link
Member Author

tlrx commented Apr 4, 2018

Thanks @nik9000 and @rjernst

martijnvg added a commit that referenced this pull request Apr 5, 2018
* es/master: (68 commits)
  Allow using distance measure in the geo context precision (#29273)
  Disable failing query in QueryBuilderBWCIT.
  Fixed quote_field_suffix in query_string (#29332)
  Use fixture to test repository-url module (#29355)
  Remove undocumented action.master.force_local setting (#29351)
  Enhance error for out of bounds byte size settings (#29338)
  Fix QueryAnalyzerTests.
  Fix HasChildQueryBuilderTests to not use the `classic` similarity.
  [Docs] Correct javadoc of GetIndexRequest (#29364)
  Make TransportRankEvalAction members final
  Add awaits fix for a query analyzer test
  Check presence of multi-types before validating new mapping (#29316)
  Add awaits fix for HasChildQueryBuilderTests
  Remove silent batch mode from install plugin (#29359)
  Align cat thread pool info to thread pool config (#29195)
  Track Lucene operations in engine explicitly (#29357)
  Build: Fix Java9 MR build (#29312)
  Reindex: Fix error in delete-by-query rest spec (#29318)
  Improve similarity integration. (#29187)
  Fix some query extraction bugs. (#29283)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >test Issues or PRs that are addressing/adding tests v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants