Synchronize files from linux-system-roles/template #133

systemroller · 2019-11-20T13:46:49Z

This PR propagates files from linux-system-roles/template which should be in sync across linux-system-roles repos. In case of changing affected files via pushing to this PR, please do not forget also to push the changes to linux-system-roles/template repo.

CC: @i386x, @pcahyna.

pcahyna · 2019-11-20T16:12:09Z

@i386x "Files copied on wrong place " - is this a bug in the script or in the template?

tyll · 2019-11-20T16:16:41Z

The custom command fails on Travis: https://travis-ci.com/linux-system-roles/network/jobs/258652534#L272

tyll · 2019-11-20T16:19:12Z

tox.ini

+  3.6: py36,black,coveralls,custom
+  3.7: py37,coveralls,custom
+  3.7-dev: py37,coveralls,custom
+  3.8-dev: py38,coveralls,custom


Is it correct to remove python 3.8 here?

I am not sure. Python 3.8.0 is the latest release of Python now and will be shortly supported in Fedora. Also it seems that it is supported by Travis, so I propose to add 3.8 line besides 3.8-dev line.

do we need both 3.7-dev and 3.8-dev ? also, do we need coveralls in multiple places?

merged the python version change in #137 for travis.yml. 3.8-dev does not need to be mentioned here (not sure if it really works) because 3.8 also matches the development version for 3.8:
https://travis-ci.com/linux-system-roles/network/jobs/263414315

pcahyna · 2019-11-20T16:28:42Z

The custom command fails on Travis: https://travis-ci.com/linux-system-roles/network/jobs/258652534#L272

.travis/custom.sh: line 26: .tox/env-3.6/bin/python: No such file or directory
maybe it needs python3 instead of python?

i386x · 2019-11-21T06:10:45Z

@i386x "Files copied on wrong place " - is this a bug in the script or in the template?

This was a bug in sync script. It should be fixed now.

i386x · 2019-11-21T10:10:01Z

The custom command fails on Travis: https://travis-ci.com/linux-system-roles/network/jobs/258652534#L272

.travis/custom.sh: line 26: .tox/env-3.6/bin/python: No such file or directory
maybe it needs python3 instead of python?

This puzzles me most, the absolute path /home/travis/build/linux-system-roles/network/.tox/env-3.6/bin/python, passed to custom.sh script as 1st argument, was shortened to .tox/env-3.6/bin/python in some way, which results in the mentioned error, because .tox/env-3.6/bin/python cannot be seen from tests/ directory.

coveralls · 2019-11-21T11:23:39Z

Pull Request Test Coverage Report for Build 398

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
Change from base Build 391:	0.0%
Covered Lines:
Relevant Lines:	0

💛 - Coveralls

pcahyna · 2019-11-21T11:58:50Z

.travis/custom.sh

@@ -23,5 +23,5 @@ shift 2

 # Write your custom commands here that should be run when `tox -e custom`:
 if [[ -z "${TRAVIS}" ]] || lsr_check_python_version ${ENVPYTHON} -eq '3.6'; then
-  (set -x; cd ${TOPDIR}/tests && ${ENVPYTHON} ./ensure_non_running_provider.py)
+  (set -x; cd ${TOPDIR}/tests; ${ENVPYTHON} ./ensure_non_running_provider.py)


why this change? you don't care whether cd has succeeded or failed?

set -e should handle this (exit immediately if a command exits with a non-zero status).

pcahyna · 2019-11-21T12:01:43Z

.travis/runcoveralls.sh

-mv ${TOPDIR}/.coverage ${TOPDIR}/.coverage.merge
-${ENVPYTHON} -m coverage combine --append ${TOPDIR}
+mv ${TOPDIR}/.coverage ${TOPDIR}/.coverage.merge || :
+${ENVPYTHON} -m coverage combine --append ${TOPDIR} || :


@tyll can you please review the Coveralls bits?

i386x · 2019-11-21T15:52:44Z

The custom command fails on Travis: https://travis-ci.com/linux-system-roles/network/jobs/258652534#L272

.travis/custom.sh: line 26: .tox/env-3.6/bin/python: No such file or directory
maybe it needs python3 instead of python?

This puzzles me most, the absolute path /home/travis/build/linux-system-roles/network/.tox/env-3.6/bin/python, passed to custom.sh script as 1st argument, was shortened to .tox/env-3.6/bin/python in some way, which results in the mentioned error, because .tox/env-3.6/bin/python cannot be seen from tests/ directory.

Not bash, but tox-dev/tox@43d65b17#diff-57c0f90ac19d5a79d6495ab22fab6539R247-R258 is the source of problem (luckily, workaround should be easy).

pcahyna · 2019-11-21T16:00:12Z

The custom command fails on Travis: https://travis-ci.com/linux-system-roles/network/jobs/258652534#L272

.travis/custom.sh: line 26: .tox/env-3.6/bin/python: No such file or directory
maybe it needs python3 instead of python?

This puzzles me most, the absolute path /home/travis/build/linux-system-roles/network/.tox/env-3.6/bin/python, passed to custom.sh script as 1st argument, was shortened to .tox/env-3.6/bin/python in some way, which results in the mentioned error, because .tox/env-3.6/bin/python cannot be seen from tests/ directory.

Not bash, but tox-dev/tox@43d65b1#diff-57c0f90ac19d5a79d6495ab22fab6539R247-R258 is the source of problem (luckily, workaround should be easy).

Is this behavior of tox documented? Should it be reported as a bug if not?

i386x · 2019-11-21T19:51:33Z

Is this behavior of tox documented? Should it be reported as a bug if not?

I file issue on tox-dev/tox for it: tox-dev/tox#1463

pcahyna · 2019-11-22T19:05:27Z

Is this behavior of tox documented? Should it be reported as a bug if not?

I file issue on tox-dev/tox for it: tox-dev/tox#1463

Why is conversion of absolute to relative path even a problem? The result should be equivalent, no?

i386x · 2019-11-23T18:04:09Z

Why is conversion of absolute to relative path even a problem? The result should be equivalent, no?

The problem is a little bit complex. Seeing the logs, sometimes the path to virtual env interpreter is absolute and sometimes it is relative. Moreover, the path shortening feature can be changed or reverted in the future so it is good to not rely on it.

tyll · 2019-11-27T13:19:28Z

.travis/runblack.sh

+shift
+
+DEFAULT_INCLUDE='^[^.].*\.py$'
+DEFAULT_EXCLUDE='/(\.[^.].*|tests/roles)/'


I would prefer to run black for the python scripts in the tests dir as well.

Python scripts in tests/ should be analyzed by black. The tests/roles part of regexp excludes directory containing symlinks.

.travis.yml

.lgtm.yml

tyll · 2019-11-27T19:57:35Z

I merged some changes in #136 to demonstrate what I would wish the commits look like.

tyll · 2019-11-27T19:59:05Z

.travis/runtox

-tox "$@"
-for X in ${LSR_MSCENARIOS}; do
-  LSR_MSCENARIO=${X} tox -e molecule
+for X in ${LSR_ANSIBLES}; do


IMHO it would be better to avoid variables like X and Y and use for example ansible_dependency and scenario.

tyll · 2019-11-29T13:55:32Z

.travis/preinstall

+# Include library.
+. ${SCRIPTDIR}/utils.sh
+
+# Add python3-selinux package (it turned out that in some circumstances, i.e


AFAIU this is because molecule is using the copy or file Ansible modules on a selinux enabled system to setup the container. It has nothing to do with the tests itself but with setting up the test environment.

Thank You for noting this, I will mention it in the comment.

tyll · 2019-11-29T13:57:02Z

.travis/preinstall

 fi

+# Include config.


Actually the comments about including config or utils could be removed because the actual code to do this is simple enough that the comments do not explain something new.

.travis/runtox

This prevents a surprise when default building environment bumps.

tyll · 2019-12-05T10:36:07Z

.travis.yml

@@ -2,6 +2,10 @@
 ---
 dist: xenial
 language: python
+env:
+  global:
+    - LSR_ANSIBLES='ansible==2.6.* ansible==2.7.* ansible==2.8.*'


Why is this configured in travis.yml and not in config.sh? In case somone wants to run molecule outside of travis, this needs to be defined as well, doesn't it? This also affects the variable on the next line.

config.sh should contain only parts that may vary across repositories. Running molecule outside Travis can be performed via LSR_ANSIBLE_DEP=<ansible-dep> LSR_MSCENARIO=<molecule scenario> tox -e molecule (just tox -e molecule runs molecule with the newest Ansible Python package installed and with the default scenario). Furthermore, running molecule for all supported Ansible versions and for all supported scenarios outside of Travis by one command will be imho possible only by introducing a shell script that wraps tox command (this possibility is worth of discussion). Maybe it should be good to write a documentation for our Travis and tox based CI and put it to file named like TOX.INI.README.md.

tyll · 2019-12-05T10:38:17Z

.travis/runblack.sh

+#!/bin/bash
+# SPDX-License-Identifier: MIT
+
+# Run black (the Python formatter). The first script argument is a path to


Can you please add the reason why we need this wrapper to the comment?

Done in #145.

tyll · 2019-12-05T10:38:33Z

.travis/runflake8.sh

+#!/bin/bash
+# SPDX-License-Identifier: MIT
+
+# Run flake8. The first script argument is a path to Python interpreter, the


Can you please add the reason why we need this wrapper to the comment?

Done in #147.

tyll · 2019-12-05T10:38:39Z

.travis/runpytest.sh

+#!/bin/bash
+# SPDX-License-Identifier: MIT
+
+# Wrapper around pytest. First argument is a path to environment python, the


Can you please add the reason why we need this wrapper to the comment?

Done in #144.

i386x · 2020-02-18T23:16:14Z

Closing this issue as it has been resolved by #142, #144, #145, #146, #147, #148, #149, and #151.

tyll reviewed Nov 20, 2019

View reviewed changes

pcahyna reviewed Nov 21, 2019

View reviewed changes

tyll reviewed Nov 27, 2019

View reviewed changes

.travis.yml Show resolved Hide resolved

tyll reviewed Nov 27, 2019

View reviewed changes

.lgtm.yml Outdated Show resolved Hide resolved

tyll mentioned this pull request Nov 27, 2019

Break long lines in defaults and remove deprecated checks #135

Merged

tyll force-pushed the lsr-template-sync branch 3 times, most recently from 24d8352 to bedf279 Compare November 27, 2019 18:31

tyll reviewed Nov 27, 2019

View reviewed changes

tyll reviewed Nov 29, 2019

View reviewed changes

tyll reviewed Dec 3, 2019

View reviewed changes

.travis/runtox Show resolved Hide resolved

systemroller and others added 4 commits December 5, 2019 11:30

🤖 synchronize files from linux-system-roles/template

aaaf204

Add custom settings

8851c65

Note Ubuntu distro explicitly

c07def3

This prevents a surprise when default building environment bumps.

Remove 3.7-dev, add 3.8

0d1d1d8

i386x added 3 commits December 5, 2019 11:30

Use meaningful variable names

342d666

Better explain python3-selinux requirement

ff13910

Remove meaningless comments

c19eb5e

tyll force-pushed the lsr-template-sync branch from c7e05a3 to c19eb5e Compare December 5, 2019 10:33

tyll reviewed Dec 5, 2019

View reviewed changes

This was referenced Jan 20, 2020

Onboard refactored tox & Travis CI setup and configuration vol. 5 #147

Merged

Onboard refactored tox & Travis CI setup and configuration vol. 6 #148

Merged

Onboard refactored tox & Travis CI setup and configuration vol. 7 #149

Merged

This was referenced Jan 31, 2020

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

Merged

Onboard refactored tox & Travis CI setup and configuration vol. 9 #152

Closed

i386x closed this Feb 18, 2020

i386x deleted the lsr-template-sync branch February 18, 2020 23:27

Synchronize files from linux-system-roles/template #133

Synchronize files from linux-system-roles/template #133

Conversation

systemroller commented Nov 20, 2019

pcahyna commented Nov 20, 2019

tyll commented Nov 20, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tyll Dec 5, 2019 • edited Loading

Choose a reason for hiding this comment

pcahyna commented Nov 20, 2019

i386x commented Nov 21, 2019

i386x commented Nov 21, 2019

coveralls commented Nov 21, 2019 • edited Loading

Pull Request Test Coverage Report for Build 398

💛 - Coveralls

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

i386x commented Nov 21, 2019

pcahyna commented Nov 21, 2019

i386x commented Nov 21, 2019

pcahyna commented Nov 22, 2019

i386x commented Nov 23, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tyll commented Nov 27, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

i386x commented Feb 18, 2020

tyll Dec 5, 2019 •

edited

Loading

coveralls commented Nov 21, 2019 •

edited

Loading