Explicitly specify the name of the container running tests in Jenkins #316
Conversation
This file contains 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
|
@brainzbot retest this please |
alastair
requested changes
Jan 17, 2019
docker/jenkins-test.sh
Outdated
| @@ -46,14 +46,14 @@ function run_tests { | |||
|
|
|||
| # create database and user once | |||
| echo "creating user and database" | |||
| docker-compose -f $COMPOSE_FILE_LOC -p $COMPOSE_PROJECT_NAME run --rm $TEST_CONTAINER_NAME \ | |||
| docker-compose -f $COMPOSE_FILE_LOC -p $COMPOSE_PROJECT_NAME run --rm $TEST_CONTAINER_NAME --name $TEST_CONTAINER_REF\ | |||
There was a problem hiding this comment.
I don't think we need --name here - we're already using --rm. This container only runs init_db and then quits and removes itself.
docker/jenkins-test.sh
Outdated
| dockerize \ | ||
| -wait tcp://db:5432 -timeout 60s bash -c \ | ||
| "cd /code && cp config.py.example config.py && ls -lR && python manage.py init_db" | ||
|
|
||
| # run tests | ||
| echo "running tests" | ||
| docker-compose -f $COMPOSE_FILE_LOC -p $COMPOSE_PROJECT_NAME run $TEST_CONTAINER_NAME \ | ||
| docker-compose -f $COMPOSE_FILE_LOC -p $COMPOSE_PROJECT_NAME run $TEST_CONTAINER_NAME --name $TEST_CONTAINER_REF \ |
There was a problem hiding this comment.
Should we add a comment saying why we explicitly set a name? (so that we have a consistent name so that we can use docker cp later in the script)
The only thing we have to be sure about here is that the containers gets removed properly after it runs, and that different runs of the tests don't conflict with each other. I think that we're OK here:
- the project name contains the jenkins tag, which increases for each time that jenkins runs a job (and this is always how it's run in the past, without problems)
- the cleanup function stops and removes all containers with the project name, so this should be removed as well with no problems
alastair
approved these changes
Jan 17, 2019
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.
No description provided.