Add docker-compose fixtures for S3 integration tests #49107
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tlrx
added
:Delivery/Build
|
Pinging @elastic/es-distributed (:Distributed/Snapshot/Restore) |
|
Pinging @elastic/es-core-infra (:Core/Infra/Build) |
alpar-t
reviewed
Nov 15, 2019
| @@ -52,7 +52,7 @@ dependencies { | |||
| // and whitelist this hack in JarHell | |||
| compile 'javax.xml.bind:jaxb-api:2.2.2' | |||
|
|
|||
| testCompile project(':test:fixtures:minio-fixture') | |||
| testCompile project(':test:fixtures:s3-fixture') | |||
There was a problem hiding this comment.
would be nice to be able to avoid this, but as we discussed in one of the other PRs we can do that separately.
There was a problem hiding this comment.
What's the concern about having this additional test dependency?
There was a problem hiding this comment.
@mark-vieira We discussed this point in #48636 (comment).
There was a problem hiding this comment.
Thanks @mark-vieira for your feedback 👍
mark-vieira
approved these changes
Nov 15, 2019
| @@ -52,7 +52,7 @@ dependencies { | |||
| // and whitelist this hack in JarHell | |||
| compile 'javax.xml.bind:jaxb-api:2.2.2' | |||
|
|
|||
| testCompile project(':test:fixtures:minio-fixture') | |||
| testCompile project(':test:fixtures:s3-fixture') | |||
There was a problem hiding this comment.
What's the concern about having this additional test dependency?
original-brownbear
approved these changes
Nov 15, 2019
|
Thanks @atorok, @mark-vieira and @original-brownbear! I'm happy to see this change merged :) |
tlrx
added a commit
to tlrx/elasticsearch
that referenced
this pull request
Nov 18, 2019
Similarly to what has been done for Azure (elastic#48636) and GCS (elastic#48762), this committ removes the existing Ant fixture that emulates a S3 storage service in favor of multiple docker-compose based fixtures. The goals here are multiple: be able to reuse a s3-fixture outside of the repository-s3 plugin; allow parallel execution of integration tests; removes the existing AmazonS3Fixture that has evolved in a weird beast in dedicated, more maintainable fixtures. The server side logic that emulates S3 mostly comes from the latest HttpHandler made for S3 blob store repository tests, with additional features extracted from the (now removed) AmazonS3Fixture: authentication checks, session token checks and improved response errors. Chunked upload request support for S3 object has been added too. The server side logic of all tests now reside in a single S3HttpHandler class. Whereas AmazonS3Fixture contained logic for basic tests, session token tests, EC2 tests or ECS tests, the S3 fixtures are now dedicated to each kind of test. Fixtures are inheriting from each other, making things easier to maintain.
tlrx
added a commit
that referenced
this pull request
Nov 18, 2019
Similarly to what has been done for Azure (#48636) and GCS (#48762), this committ removes the existing Ant fixture that emulates a S3 storage service in favor of multiple docker-compose based fixtures. The goals here are multiple: be able to reuse a s3-fixture outside of the repository-s3 plugin; allow parallel execution of integration tests; removes the existing AmazonS3Fixture that has evolved in a weird beast in dedicated, more maintainable fixtures. The server side logic that emulates S3 mostly comes from the latest HttpHandler made for S3 blob store repository tests, with additional features extracted from the (now removed) AmazonS3Fixture: authentication checks, session token checks and improved response errors. Chunked upload request support for S3 object has been added too. The server side logic of all tests now reside in a single S3HttpHandler class. Whereas AmazonS3Fixture contained logic for basic tests, session token tests, EC2 tests or ECS tests, the S3 fixtures are now dedicated to each kind of test. Fixtures are inheriting from each other, making things easier to maintain.
tlrx
added a commit
that referenced
this pull request
Nov 18, 2019
Similarly to what has been done for Azure (#48636) and GCS (#48762), this committ removes the existing Ant fixture that emulates a S3 storage service in favor of multiple docker-compose based fixtures. The goals here are multiple: be able to reuse a s3-fixture outside of the repository-s3 plugin; allow parallel execution of integration tests; removes the existing AmazonS3Fixture that has evolved in a weird beast in dedicated, more maintainable fixtures. The server side logic that emulates S3 mostly comes from the latest HttpHandler made for S3 blob store repository tests, with additional features extracted from the (now removed) AmazonS3Fixture: authentication checks, session token checks and improved response errors. Chunked upload request support for S3 object has been added too. The server side logic of all tests now reside in a single S3HttpHandler class. Whereas AmazonS3Fixture contained logic for basic tests, session token tests, EC2 tests or ECS tests, the S3 fixtures are now dedicated to each kind of test. Fixtures are inheriting from each other, making things easier to maintain.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Similarly to what has been done for Azure (#48636) and GCS (#48762), this pull request removes the existing Ant fixture that emulates a S3 storage service in favor of multiple docker-compose based fixtures.
The goals here are multiple:
s3-fixtureoutside of therepository-s3plugin;AmazonS3Fixturethat has evolved in a weird beast in dedicated, more maintainable fixtures.The server side logic that emulates S3 mostly comes from the latest HttpHandler made for S3 blob store repository tests, with additional features extracted from the (now removed)
AmazonS3Fixture:I also had to add chunked upload request support for S3 object as it was not fully supported. The server side logic of all tests now reside in a single
S3HttpHandlerclass.Whereas
AmazonS3Fixturecontained logic for basic tests, session token tests, EC2 tests or ECS tests, the S3 fixtures are now dedicated to each kind of test. It's also now to see that fixtures are inheriting from each other, making things easier to maintain or understand (at least to me).While this change makes things easier to reuse and maintain, I think it has an impact on test execution time but I think it worth it.