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

Fix running test suite under Docker Machine #257

Merged
merged 9 commits into from
Oct 13, 2015

Conversation

md5
Copy link
Contributor

@md5 md5 commented Oct 11, 2015

This PR makes the test suite run correctly under Docker Machine, including under OS X.

Fixes include:

@md5
Copy link
Contributor Author

md5 commented Oct 11, 2015

Looks like my appropriate/curl image locally differed from the one on Docker Hub... Lemme fix that.

@md5
Copy link
Contributor Author

md5 commented Oct 11, 2015

I pushed a fix for appropriate/curl here: appropriate/docker-curl@af2cc9f

I also pushed a new commit to this branch to remove a commented out line that should trigger a (hopefully successful) test run.

@md5
Copy link
Contributor Author

md5 commented Oct 11, 2015

Now there seems to be some sort of error with Circle CI...

Error deleting container: Error response from daemon: Cannot destroy container a8bc77c4639069aebb72210eeadc18b6b50fa24c3f70670547cfc3031a92c38b: Driver btrfs failed to remove root filesystem a8bc77c4639069aebb72210eeadc18b6b50fa24c3f70670547cfc3031a92c38b: Failed to destroy btrfs snapshot: operation not permitted

@md5
Copy link
Contributor Author

md5 commented Oct 12, 2015

BTW, reading the diff for this PR is much easier if whitespace is ignored since I changed the line endings on two of the files from DOS to Unix: https://github.com/jwilder/nginx-proxy/pull/257/files?w=1

@jwilder
Copy link
Collaborator

jwilder commented Oct 12, 2015

I'd like to merge this but, I have not been able to get circleci to run the tests succesfully yet. Seems to be complaining about btrfs snapshots which might be related to this: https://github.com/jwilder/nginx-proxy/pull/257/files#diff-29944324a3cbf9f4bd0162dfe3975d88R18.

@md5
Copy link
Contributor Author

md5 commented Oct 12, 2015

I don't think that the failure is related to docker pull docker. I'm pretty sure it's coming from the docker_clean helper here: https://github.com/appropriate/nginx-proxy/blob/osx-fix-test-suite/test/lib/docker_helpers.bash#L7

I'm not sure why it suddenly started being reported as a problem, however.

md5 added 2 commits October 12, 2015 20:59
Trying to avoid "Failed to destroy btrfs snapshot" errors on CircleCI
@md5
Copy link
Contributor Author

md5 commented Oct 13, 2015

Alright @jwilder

Got it green with a couple of changes! 💸 💚

jwilder added a commit that referenced this pull request Oct 13, 2015
Fix running test suite under Docker Machine
@jwilder jwilder merged commit f819a4e into nginx-proxy:master Oct 13, 2015
@jwilder
Copy link
Collaborator

jwilder commented Oct 13, 2015

Nice @md5! Thanks for this PR!

@md5 md5 deleted the osx-fix-test-suite branch October 13, 2015 04:50
@md5
Copy link
Contributor Author

md5 commented Oct 13, 2015

Sure thing. It might be nice to still have the --rm when not running on CircleCI. I may have another PR for that at some point, since I don't like how many containers this leaves around when I test it locally.

I've also got another PR that I'm going to submit in the next few minutes that cuts the test time in half. Hopefully CircleCI doesn't choke on it...

@thomasleveil
Copy link
Contributor

I left the test containers hanging around on purpose to allow manual tests and ease investigating a failing test case.

My workflow is:

  • start the containers for the failing test case
  • do the curl queries to reproduce the issue and find a fix

we could provide a shell script to remove all containers named 'bats-*' to clean up after bats if no failing case were found

@md5
Copy link
Contributor Author

md5 commented Oct 13, 2015 via email

@thomasleveil
Copy link
Contributor

my mistake, then what about cleaning them in the bats teardown function?

For instance, they could be started with the docker label bats-type=teardown and the teardown function would be:

function teardown {
    CIDS=( $(docker ps -q --filter "label=bats-type=teardown") )
    if [ ${#CIDS[@]} -gt 0 ]; then
            docker rm -vf ${CIDS[@]} >&2
    fi
}

@md5
Copy link
Contributor Author

md5 commented Oct 13, 2015 via email

Alexander-Krause-Glau pushed a commit to Alexander-Krause-Glau/rpi-docker-nginx-proxy that referenced this pull request Mar 30, 2018
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.

3 participants