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

Improve buildkite test_containers.sh by not erroring if docker rm/rmi --fail #1016

Merged
merged 2 commits into from
Jul 30, 2018

Conversation

rfay
Copy link
Member

@rfay rfay commented Jul 26, 2018

The Problem/Issue/Bug:

  • There seem to be times on the buildkite testbots where docker rm or docker rmi can fail... but we're only doing it in the context of cleaning up anyway, so it shouldn't cause the tests to fail.
  • Docker images should be built in testing with --no-cache. Turns out build-tools has a DOCKER_ARGS makefile variable for this exact purpose.

How this PR Solves The Problem:

  • Ignore failure on docker rm.
  • Add --no-cache to container build in circle and buildkite tests.

@rfay rfay added this to the v1.1.0 milestone Jul 26, 2018
@rfay rfay self-assigned this Jul 26, 2018
@rfay rfay requested a review from andrewfrench July 26, 2018 18:35
@rfay
Copy link
Member Author

rfay commented Jul 27, 2018

The premise here was false, I was not analyzing the behavior of the failures very well. However I think the script improvements do no harm, and certainly don't affect the end-user.

The big argument is to use BATS to build readable tests on our container tests with parseable test results.

Copy link
Contributor

@andrewfrench andrewfrench left a comment

Choose a reason for hiding this comment

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

Changes look reasonable to me (even if this didn't solve the original issue) and the tests agree, good to go.

@rfay rfay merged commit 9f5eca4 into ddev:master Jul 30, 2018
@rfay rfay deleted the 20180726_soften_cleanup branch July 30, 2018 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants