-
Notifications
You must be signed in to change notification settings - Fork 16
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
feat: Containerize integration tests. #809
Conversation
Looks like I got my branch names mixed up. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Referring back to the acceptance criteria in SYNC-4118 there are some things missing in this PR:
- Adding a Make command
- Updating the CircleCI to build the container and execute the containerized tests via the make command. Also does SRE need the Docker image to be pushed if these are also running in CD?
- Updating the testing.md documentation.
I left the old integration test makefile command as it is easier to run specific tests that way. Passing arguments through to the docker image is painful and I haven't been able to get it to work reliably. |
@@ -354,6 +329,26 @@ jobs: | |||
paths: | |||
- autopush-locust.tar | |||
|
|||
build-integration-test: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for adding the CI stuff. It all works great locally for me! 🌟
Double checking:
I think this job needs to be added to the workflow? Can the image be re-used by the integration test job as a pre-req?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I didn't add it as I wanted to do it in a separate PR. It could be reused after it's pushed to GAR. That would allow us to use circleci layer caching too actually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with understanding that there is a follow-up PR coming to finish the integration with CI. Thank you @b4handjr
This adds the needed files to containerize the integration tests. It can be run as follows:
docker compose -f tests/integration/docker-compose.yml run -it integration-tests
.Fixes: SYNC-4118