-
-
Notifications
You must be signed in to change notification settings - Fork 782
Switch travis to use Bionic, Mongo 4.0 #4862
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
Conversation
e8ba30d to
85262da
Compare
blag
left a comment
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.
Review while WIP to point out alternative changes that should hopefully make your life easier.
6d7c8d3 to
86e229a
Compare
bb47419 to
2863e4b
Compare
2863e4b to
e957202
Compare
|
Mongo 4.0 significantly changes the test runtimes. 2x was too much of a 24% increase for "Unit Tests (Python 2.7 MongoDB 4.0)" (812s) Note that this PR also fixes the Python 3.6 integration tests to When we move to python 3.7 that will probably decrease the py3 times. |
|
This is ready. I updated the PR description to better cover the updates required in moving to Bionic. |
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.
According to timing stats you provided, we'd likely go with the #4863 Xenial + Mongo 3.4 for the st2 Unit + Integration tests.
It's really good to have the choice here, so thanks for the extended testing and timing breakdown! 👍
blag
left a comment
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.
Still have some questions.
| if [ "${TASK}" = 'compilepy3 ci-py3-unit' ] || [ "${TASK}" = 'ci-py3-integration' ]; then | ||
| pip install "tox==3.8.6" | ||
|
|
||
| # cleanup any invalid python2 cache |
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.
Whitespace issues here - using tabs when the rest of the file uses spaces.
| if [ "${UBUNTU_VERSION}" == "xenial" ]; then | ||
| echo "Applying workaround for stanley user permissions issue to /home/travis on Xenial" | ||
| chmod 777 -R /home/travis | ||
| if [ "${UBUNTU_VERSION}" == "xenial" ] || [ "${UBUNTU_VERSION}" == "bionic" ]; then |
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.
If you duplicated this logic into the new scripts/travis/permissions-workaround.sh, then I would remove this code here, and simply call that script here. Alternatively, you could use make magic to build both targets.
| # code and runs tests under a different system user). | ||
| if [ "${UBUNTU_VERSION}" == "xenial" ] || [ "${UBUNTU_VERSION}" == "bionic" ]; then | ||
| echo "Applying workaround for stanley user permissions issue to /home/travis on Xenial/Bionic" | ||
| chmod -R a+rwX /home/travis |
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.
Would it make more sense to change the ownership of some of the files in /home/travis to stanley instead of trying to chmod them to the appropriate settings?
And even if it still makes sense to use chmod, we still should not be setting all of the bits. We should be as selective as possible.
No longer needed without the old xenial permissions workaround that set the entire /home/travis with 777 perms.
The Makefile defaults PYTHON_VERSION to python2.7 so the virtualenv was building with python2.7 even in the 3.6 tests, and then it was cached for future test runs with python2.7. This resolves both of those issues.
The .circle/add-itest-user.sh was actually running for all tasks, so this removes the if statement. The permissions workaround needed to run for packs-tests and integration tests, but there was no easy way to target those unless we separate the py3 packs-tests from the py3 unit tests. That also makes it much easier to reason about the different travis environments to understand how they map to make targets and tox envs. That also made it clear that one of the tox envs was not getting pre-built, so this adjusts install-requirements.sh to account for it.
The timing of several make targets is higher than the threshold. For ci-py3-integration, it looks like that threshold is far too short for what the tests regularly take. So, this adjust the thresholds to be about 60s more than the last successful build in travis. make target: orig threshold=>new threshold (reference duration) ci-unit: 700=>740 (686) ci-integration: 700=>740 (683) ci-checks ci-packs-tests: 280=>440 (381) ...-py3-unit ci-py3-pac...: 680=>740 (689) ci-py3-integration: 310=>720 (660)
Mongo 4.0 significantly changes the test runtimes. 2x was too much of a jump, so this is approximately 60s more than the last successful run time. That comes out to (test time used to calculate in parentheses): 24% increase for "Unit Tests (Python 2.7 MongoDB 4.0)" (812s) 37% increase for "Integration Tests (Python 2.7)" (903s) 95% increase for "Lint Checks, Packs Tests (Python 2.7)" (490s) 71% increase for "Unit Tests, Pack Tests (Python 3.6)" (1101s) 252% increase for "Integration Tests (Python 3.6)" (1031s) Note that this PR also fixes the Python 3.6 integration tests to actually run in python 3.6 instead of 2.7, so that might account for some of the significant test time increase. When we move to python 3.7 that will probably decrease the py3 times.
e957202 to
6cced4b
Compare
Switches travis to use Bionic with Mongo 4.0
Mongo 3.4 is not available in Bionic, so 4.0 is required.
Also, the test thresholds were increased to account for increased test times due to the mongo update.