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 TestFixturesPlugin to Run Minio in Tests (#37852) #38973

Merged
merged 8 commits into from Mar 11, 2019

Conversation

talevy
Copy link
Contributor

@talevy talevy commented Feb 15, 2019

@talevy talevy added >test Issues or PRs that are addressing/adding tests :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v6.6.0 backport labels Feb 15, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

* Use TestFixturesPlugin to Run Minio in Tests

* Closes elastic#37680
* Closes elastic#37783
@talevy
Copy link
Contributor Author

talevy commented Feb 15, 2019

run elasticsearch-ci/1

1 similar comment
@talevy
Copy link
Contributor Author

talevy commented Feb 15, 2019

run elasticsearch-ci/1

@talevy
Copy link
Contributor Author

talevy commented Feb 15, 2019

run elasticsearch-ci/1

@talevy
Copy link
Contributor Author

talevy commented Feb 16, 2019

doesn't look like this backported cleanly. can check next week

@original-brownbear
Copy link
Member

@talevy maybe this is missing: #37878? (let me know if you need any help here + thanks for taking this on!)

talevy and others added 2 commits February 19, 2019 08:00
* Disable Minio fixture and tests that require it when fixtures are disabled or Docker is not available
* Relates elastic#37852
@talevy
Copy link
Contributor Author

talevy commented Feb 19, 2019

oh, ok. I've merge that into here so we find out. thank you @original-brownbear!

@talevy
Copy link
Contributor Author

talevy commented Feb 19, 2019

run elasticsearch-ci/1

@talevy
Copy link
Contributor Author

talevy commented Feb 19, 2019

@original-brownbear I might need help here... 😄 I included the PR you mentioned, and it fails in the same way

@original-brownbear original-brownbear self-assigned this Feb 19, 2019
@original-brownbear
Copy link
Member

@talevy sure, will take a look tomorrow :)

description = "Runs REST tests using the Minio repository."
}

preProcessFixture.dependsOn(writeDockerFile)
Copy link
Member

Choose a reason for hiding this comment

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

@atorok you may be able to help here better than me :) Is there anything different about this task in 6.6 maybe that would prevent this way of triggering the docker file write from working? It seems this task isn't executed before composeUp and thus composeUp fails.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't usually back port build changes this far so I'm fairly sure there's some change to the plugin missing in this branch.

Copy link
Member

Choose a reason for hiding this comment

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

Ah yea that makes sense, I guess we could just make composeUp depend on writeDockerFile as a workaround in 6.6?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

@talevy ah sorry missed your ping. Yea but I remember there was some weirdness around this actually happening before fixes by Alpar => let's try triggering on composeUp :)

@jpountz jpountz added v6.6.3 and removed v6.6.2 labels Mar 7, 2019
@talevy
Copy link
Contributor Author

talevy commented Mar 8, 2019

thank you for the help Armin. to be honest, I haven't been keeping up with build
failures, so I am not sure if this is still an issue in 6.6... but the build went green!

Copy link
Member

@original-brownbear original-brownbear left a comment

Choose a reason for hiding this comment

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

LGTM, since we don't have any production code changes here + the failure is def. still a possibility I think it's fine to go ahead with this still

@talevy
Copy link
Contributor Author

talevy commented Mar 9, 2019

thanks Armin! I'll run the tests again just to add more confidence

@talevy
Copy link
Contributor Author

talevy commented Mar 9, 2019

run elasticsearch-ci/1

@talevy talevy merged commit 8127693 into elastic:6.6 Mar 11, 2019
@talevy talevy deleted the backport-37852 branch March 11, 2019 05:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >test Issues or PRs that are addressing/adding tests v6.6.3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants