-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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: add timeouts on docker local-invoke, start-api and start-lambda integration tests #1478
Conversation
- cause the builds to fail on appveyor rather than halting the test forever, resulting in a lengthy build timeout.
Might need to adopt https://github.com/pytest-dev/pytest-rerunfailures as well. |
@@ -1,13 +1,13 @@ | |||
coverage==4.3.4 | |||
tox==2.2.1 |
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.
No we don't need tox. +1
pytest-cov==2.4.0 | ||
# astroid > 2.0.4 is not compatible with pylint1.7 | ||
astroid>=1.5.8,<2.1.0 | ||
pylint==1.7.2 | ||
|
||
# Test requirements | ||
pytest==3.1.0 | ||
py==1.4.33 | ||
pytest==3.6.0 |
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.
This might cause some weird test failures/warnings I think.
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.
Are you talking about removing py or the pytest upgrade?
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.
I had to upgrade pytest to even be able to use pytest-timeouts.
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.
LGTM. Hope tests pass
@@ -30,6 +31,7 @@ def setUp(self): | |||
def tearDown(self): | |||
os.remove(self.events_file_path) | |||
|
|||
@pytest.mark.timeout(timeout=300, method='thread') |
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.
is this seconds?
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 its seconds.
Gonna increase the lambda timeouts on the integration test templates. Looks like 3 seconds is too low (sporadically) when running docker linux containers on appveyor windows. |
- removal of python2 matrix - change to always activation of virtualenv on windows on every stage
- infinite timeout, but each of these tests has an individual timeout of 300s
55c5426
to
a1e2635
Compare
a1e2635
to
11c569c
Compare
- temporarily remove smoke tests on windows matrix
f7d22cd
to
3232aa8
Compare
forever, resulting in a lengthy build timeout.
pytest-timeout
, also do we needtox
?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.