Skip to content
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

Onboard refactored tox & Travis CI setup and configuration vol. 8 #151

Merged

Conversation

i386x
Copy link
Contributor

@i386x i386x commented Jan 31, 2020

This is an 8th batch of changes introduced by PR #133 (changes are introduced in batches to make reviews easier). List of changes introduced in this PR:

  • allow to run Molecule tests against more versions of Ansible
  • allow to run custom commands also for Python 3.8-dev
  • remove unecessary comment

@coveralls
Copy link

coveralls commented Jan 31, 2020

Pull Request Test Coverage Report for Build 441

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at ?%

Totals Coverage Status
Change from base Build 438: 0%
Covered Lines:
Relevant Lines: 0

💛 - Coveralls

.travis/runtox Outdated
lsr_banner "tox" ${BANNERSIZE}
(set -x; tox); error_code=$?

if lsr_check_python_version python -le '3.4'; then
Copy link
Member

Choose a reason for hiding this comment

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

This now tries to run molecule on Travis for all Python versions but then skips the run because the system libraries can only be accessed for 3.5. This adds a lot of noise to the logs. Technically it should be possible to run all molecule tests except for the molecule_test version with other Python versions (IIRC). Only the molecule_test actually uses the container image. But we probably do not want to do this, running it once is enough, isn't it?

Therefore I propose to also add the check for the python version here to reduce the noise. Or is this not possible because this is before tox?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for noting this. Actually, as Travis documentation says, python command here refers also to virtualenv Python, so it should be no problem to implement the check as:

if ! lsr_venv_python_matches_system_python; then
  exit $error_code
fi

@i386x i386x force-pushed the lsr-template-refactored--part-8 branch from c6507fc to 17c8b55 Compare January 31, 2020 21:17
tox.ini Outdated
3.6: py36,black,custom
3.7: py37,custom
3.8: py38,custom
3.8-dev: py38,custom
Copy link
Member

Choose a reason for hiding this comment

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

This does not seem to be needed, py38 already runs when the 3.8-dev version is installed:
https://travis-ci.com/linux-system-roles/network/jobs/281802888#L215

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I will remove it. Running custom commands only for stable Python versions should be enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 4b05c20.

Copy link
Member

Choose a reason for hiding this comment

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

Even without the line, the custom command will still run with the python 3.8 development version because 3.8 in this config also matches the development version AFAICS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even without the line, the custom command will still run with the python 3.8 development version.

I do not see the custom command in the log. However, it should be imho enough that the custom command runs in stable Python versions only.

Copy link
Member

@tyll tyll left a comment

Choose a reason for hiding this comment

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

Thank you, please remove the 3.8-dev line from tox.ini. Everything else looks ok.

Support running the molecule tests against more versions of Ansible.
By typing

  LSR_ANSIBLE_DEP=<ansible> LSR_MSCENARIO=<scenario> tox -e molecule

one can run molecule inside tox with <ansible> version installed
under <scenario> scenario. For example

  LSR_ANSIBLE_DEP='ansible==2.7.*' LSR_MSCENARIO=foo tox -e molecule

will run molecule under scenario 'foo' and the recent Ansible 2.7
installed. `tox -e molecule` run molecule under default scenario
with the latest Ansible (from pypi) installed.

Travis runs molecule tests for Ansible 2.6, 2.7, and 2.8 installed
and under default scenario (can be overriden by user in config.sh).

Additional changes: Remove useless comment from custom.sh.
@i386x i386x force-pushed the lsr-template-refactored--part-8 branch from 17c8b55 to 4b05c20 Compare February 3, 2020 11:43
Copy link
Member

@tyll tyll left a comment

Choose a reason for hiding this comment

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

Thank you, 👍

@tyll tyll merged commit d36a393 into linux-system-roles:master Feb 3, 2020
@i386x
Copy link
Contributor Author

i386x commented Feb 10, 2020

This PR reflects the changes in linux-system-roles/template@0216bef.

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

Successfully merging this pull request may close these issues.

3 participants