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
Move to using communicate with a timeout for integ tests #1571
Conversation
7bd382b
to
8cc80cf
Compare
280540f
to
476d069
Compare
appveyor-windows.yml
Outdated
@@ -75,7 +75,7 @@ test_script: | |||
- "venv\\Scripts\\activate" | |||
- "docker system prune -a -f" | |||
- "pytest -vv tests/integration" | |||
- "pytest -vv tests/regression" | |||
- "pytest -n 4 -vv tests/regression" |
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.
YES!!!!1
@@ -128,11 +118,11 @@ for: | |||
- "pylint --rcfile .pylintrc samcli" | |||
# There are some functional tests that are currently broken due to not being updated with changed code or still running with node4.3 runtimes | |||
# We need to update those but this allows us to at least runs the ones we currently have working | |||
- "pytest tests/functional/commands/validate tests/functional/commands/cli/test_global_config.py" | |||
- "pytest -n 4 tests/functional/commands/validate tests/functional/commands/cli/test_global_config.py" | |||
|
|||
# Runs only in Linux | |||
- sh: "pytest -vv tests/integration" |
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.
in the future, we should parallelize this too
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.
Agreed. I tried to do that in this PR but required some additional changes in the build tests. We need to fix this part of the tests: https://github.com/awslabs/aws-sam-cli/blob/develop/tests/integration/buildcmd/test_build_cmd.py#L72 because we validate the image was deleted. I think we did this to ensure new tests would all start with downloading images and from the same place. While I agree, this actually added mins to the test runs as well. I was going to try and run all other integ tests but build but wanted to try and keep things scoped and not solve everything at once here.
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.
Correction: That method verifies we are deleting the container not the image.
476d069
to
c4fd641
Compare
Issue #, if available:
Description of changes:
Checklist:
make pr
passesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.