Skip to content
This repository has been archived by the owner on Sep 12, 2019. It is now read-only.

More Circle CI testing #518

Closed
wants to merge 32 commits into from
Closed

More Circle CI testing #518

wants to merge 32 commits into from

Conversation

tadhg-ohiggins
Copy link
Contributor

This is a prior version of the config that worked, and I'm breaking it out into its own branch for more testing.

Copy link
Contributor

@cmc333333 cmc333333 left a comment

Choose a reason for hiding this comment

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

I'm glad to see this working! I added odds and ends throughout, but my only real request is that we squash these commits. I'm pretty sure that's possible in the merge UI, but git rebase -i HEAD~29'll do it, too.

command: |
docker create -v /usr/src/app -v /usr/src/app/.venv --name holder python:3.6.2
docker cp . holder:/usr/src/app
docker start holder
Copy link
Contributor

Choose a reason for hiding this comment

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

Very innovative solution, nice work!

- run:
name: run tests
command: |
ls eregs_libs
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this still needed?

ls eregs_libs
docker-compose -f docker-compose-circle.yml run flake8 atf_eregs
docker-compose -f docker-compose-circle.yml run py.test atf_eregs/tests
environment:
Copy link
Contributor

Choose a reason for hiding this comment

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

Not needed this time through, but we should figure out how to run the parser tests, too.

docker-compose -f docker-compose-circle.yml run flake8 atf_eregs
docker-compose -f docker-compose-circle.yml run py.test atf_eregs/tests
environment:
DEBUG: TRUE
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this needed? I think the docker-compose file is setting the DEBUG value in its containers.

@@ -13,14 +13,14 @@ if [ -n "$DEBUG" ]; then

if git diff --exit-code eregs_libs/regulations-site > /dev/null
then
git submodule update eregs_libs/regulations-site
git submodule update --init eregs_libs/regulations-site
Copy link
Contributor

Choose a reason for hiding this comment

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

The --init was added in #494 -- does this need to be rebased, maybe?

persistent_db:
image: postgres:9.4
volumes_from:
- container:holder
Copy link
Contributor

Choose a reason for hiding this comment

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

Not for this round, but if the volumes are the only significant change, what do you think about replacing them in a sed command or similar? That way we wouldn't need to maintain two configurations.

entrypoint: ls
command: ''
ports: []

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm betting this addition wasn't intended?

@tadhg-ohiggins
Copy link
Contributor Author

@cmc333333 Thanks for the comments, there's a bunch of stuff in here that's not final—I had meant to label this in progress and forgot. So I'll be removing most of what you pointed out in very small commits.

I was trying to not maintain two configurations by using docker-compose's extends functionality, but that appears to be the most recent part that broke the config.

I'll rebase before a final PR, as we definitely don't need tens of identical commit messages…

@tadhg-ohiggins
Copy link
Contributor Author

I moved the rebased commits into another branch and made a new PR. Closing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants