From 86041d991480eadd6c92db8d11b1e857bd44b816 Mon Sep 17 00:00:00 2001
From: Albin Vass <albin.vass@gmail.com>
Date: Mon, 11 May 2020 14:16:32 +0200
Subject: [PATCH] Don't require tox_envlist
Since tox_envlist has a default value it cannot be undefined
so the fail task will never run. Instead handle the case when
tox_envlist is an empty string by getting the default configured
envlist from tox. Also handle the casewhen tox_envlist is 'ALL'.
This also updates tox_install_sibling_packages to correctly
handle multiple testenvs and uses configuration supplied by
'tox --showconfig -e <envlist>' instead of guessing where the
envdir and logdir are located.
We also cannot run tox inside python because it gets complicated
to know which tox_executable we should call during the python test
cases so run these commands in ansible and pass the output to
tox_install_sibling_packages.
Since role params have higher precedence than set_fact we set an
internal _tox_envlist fact that is a comma separated list of testenvs
that should be run.
Change-Id: I9e5a1b041f653cbcff7b8ed62e4a95a0a040fdd7
---
roles/tox/README.rst | 3 +
.../library/tox_install_sibling_packages.py | 236 ++++++++++--------
roles/tox/tasks/main.yaml | 58 ++++-
roles/tox/tasks/siblings.yaml | 27 +-
test-playbooks/python/tox.ini | 12 +
test-playbooks/python/tox.yaml | 78 ++++++
test-playbooks/tox-siblings.yaml | 14 --
zuul-tests.d/python-jobs.yaml | 11 +-
8 files changed, 305 insertions(+), 134 deletions(-)
create mode 100644 test-playbooks/python/tox.ini
create mode 100644 test-playbooks/python/tox.yaml
delete mode 100644 test-playbooks/tox-siblings.yaml
diff --git a/roles/tox/README.rst b/roles/tox/README.rst
index 89cb3a499..041d4932d 100644
--- a/roles/tox/README.rst
+++ b/roles/tox/README.rst
@@ -13,6 +13,9 @@ Runs tox for a project
``ALL`` runs all test environments while an empty string runs
all test environments configured with ``envlist`` in tox.
+ Internally this will always be expanded into a comma separated
+ list of test environments to run.
+
.. zuul:rolevar:: tox_executable
:default: tox
diff --git a/roles/tox/library/tox_install_sibling_packages.py b/roles/tox/library/tox_install_sibling_packages.py
index 75a82eda7..0f89d29f2 100644
--- a/roles/tox/library/tox_install_sibling_packages.py
+++ b/roles/tox/library/tox_install_sibling_packages.py
@@ -32,9 +32,14 @@ requirements:
options:
tox_envlist:
description:
- - The tox environment to operate in.
+ - The tox test environments to act on.
required: true
type: str
+ tox_show_config:
+ description:
+ - Path to a file containing the output from C(tox --showconfig).
+ required: true
+ type: path
project_dir:
description:
- The directory in which the project we care about is in.
@@ -159,19 +164,126 @@ def _get_package_root(name, sibling_packages):
return pkg_root
+def find_installed_siblings(tox_python, package_name, sibling_python_packages):
+ installed_sibling_packages = []
+ for dep_name in get_installed_packages(tox_python):
+ log.append(
+ "Found {name} python package installed".format(
+ name=dep_name))
+ if (dep_name == package_name or
+ prAPI.to_filename(dep_name) == package_name):
+ # We don't need to re-process ourself.
+ # We've filtered ourselves from the source dir list,
+ # but let's be sure nothing is weird.
+ log.append(
+ "Skipping {name} because it's us".format(
+ name=dep_name))
+ continue
+ if dep_name in sibling_python_packages:
+ log.append(
+ "Package {name} on system in {root}".format(
+ name=dep_name,
+ root=sibling_python_packages[dep_name]))
+ installed_sibling_packages.append(dep_name)
+ elif prAPI.to_filename(dep_name) in sibling_python_packages:
+ real_name = prAPI.to_filename(dep_name)
+ log.append(
+ "Package {name} ({pkg_name}) on system in {root}".format(
+ name=dep_name,
+ pkg_name=real_name,
+ root=sibling_python_packages[real_name]))
+ # need to use dep_name here for later constraint file rewrite
+ installed_sibling_packages.append(dep_name)
+ return installed_sibling_packages
+
+
+def install_siblings(envdir, projects, package_name, constraints):
+ changed = False
+ tox_python = '{envdir}/bin/python'.format(envdir=envdir)
+
+ sibling_python_packages = get_sibling_python_packages(
+ projects, tox_python)
+ for name, root in sibling_python_packages.items():
+ log.append("Sibling {name} at {root}".format(name=name,
+ root=root))
+
+ installed_sibling_packages = find_installed_siblings(
+ tox_python,
+ package_name,
+ sibling_python_packages)
+
+ if constraints:
+ constraints_file = write_new_constraints_file(
+ constraints, installed_sibling_packages)
+
+ for sibling_package in installed_sibling_packages:
+ changed = True
+ log.append("Uninstalling {name}".format(name=sibling_package))
+ uninstall_output = subprocess.check_output(
+ [tox_python, '-m',
+ 'pip', 'uninstall', '-y', sibling_package],
+ stderr=subprocess.STDOUT)
+ log.extend(uninstall_output.decode('utf-8').split('\n'))
+
+ args = [tox_python, '-m', 'pip', 'install']
+ if constraints:
+ args.extend(['-c', constraints_file])
+
+ pkg_root = _get_package_root(sibling_package,
+ sibling_python_packages)
+ log.append(
+ "Installing {name} from {root} for deps".format(
+ name=sibling_package,
+ root=pkg_root))
+ args.append(pkg_root)
+
+ install_output = subprocess.check_output(args)
+ log.extend(install_output.decode('utf-8').split('\n'))
+
+ for sibling_package in installed_sibling_packages:
+ changed = True
+ pkg_root = _get_package_root(sibling_package,
+ sibling_python_packages)
+ log.append(
+ "Installing {name} from {root}".format(
+ name=sibling_package,
+ root=pkg_root))
+
+ install_output = subprocess.check_output(
+ [tox_python, '-m', 'pip', 'install', '--no-deps',
+ pkg_root])
+ log.extend(install_output.decode('utf-8').split('\n'))
+ return changed
+
+
def main():
module = AnsibleModule(
argument_spec=dict(
tox_envlist=dict(required=True, type='str'),
+ tox_show_config=dict(required=True, type='path'),
tox_constraints_file=dict(type='str'),
project_dir=dict(required=True, type='str'),
projects=dict(required=True, type='list'),
)
)
- envlist = module.params['tox_envlist']
constraints = module.params.get('tox_constraints_file')
project_dir = module.params['project_dir']
projects = module.params['projects']
+ tox_envlist = module.params.get('tox_envlist', '')
+ tox_show_config = module.params.get('tox_show_config')
+
+ tox_config = configparser.ConfigParser()
+ tox_config.read(tox_show_config)
+
+ envlist = {testenv.strip() for testenv
+ in tox_envlist.split(',')}
+
+ if not envlist:
+ module.exit_json(
+ changed=False,
+ msg='No envlist to run, no action needed.')
+
+ log.append('Using envlist: {}'.format(envlist))
if not os.path.exists(os.path.join(project_dir, 'setup.cfg')):
module.exit_json(changed=False, msg="No setup.cfg, no action needed")
@@ -187,110 +299,34 @@ def main():
module.exit_json(
changed=False, msg="No name in setup.cfg, skipping siblings")
- envdir = '{project_dir}/.tox/{envlist}'.format(
- project_dir=project_dir, envlist=envlist)
- if not os.path.exists(envdir):
- module.exit_json(
- changed=False, msg="envdir does not exist, skipping siblings")
-
- tox_python = '{envdir}/bin/python'.format(envdir=envdir)
- # Write a log file into the .tox dir so that it'll get picked up
- # Name it with envlist as a prefix so that fetch-tox-output will properly
- # get it in a multi-env scenario
- log_dir = '{envdir}/log'.format(envdir=envdir)
- log_file = '{log_dir}/{envlist}-siblings.txt'.format(
- log_dir=log_dir, envlist=envlist)
-
log.append(
"Processing siblings for {name} from {project_dir}".format(
name=package_name,
project_dir=project_dir))
changed = False
-
- try:
- sibling_python_packages = get_sibling_python_packages(
- projects, tox_python)
- for name, root in sibling_python_packages.items():
- log.append("Sibling {name} at {root}".format(name=name, root=root))
- found_sibling_packages = []
- for dep_name in get_installed_packages(tox_python):
- log.append(
- "Found {name} python package installed".format(
- name=dep_name))
- if (dep_name == package_name or
- prAPI.to_filename(dep_name) == package_name):
- # We don't need to re-process ourself. We've filtered ourselves
- # from the source dir list, but let's be sure nothing is weird.
- log.append(
- "Skipping {name} because it's us".format(
- name=dep_name))
- continue
- if dep_name in sibling_python_packages:
- log.append(
- "Package {name} on system in {root}".format(
- name=dep_name,
- root=sibling_python_packages[dep_name]))
- changed = True
- found_sibling_packages.append(dep_name)
- elif prAPI.to_filename(dep_name) in sibling_python_packages:
- real_name = prAPI.to_filename(dep_name)
- log.append(
- "Package {name} ({pkg_name}) on system in {root}".format(
- name=dep_name,
- pkg_name=real_name,
- root=sibling_python_packages[real_name]))
- changed = True
- # need to use dep_name here for later constraint file rewrite
- found_sibling_packages.append(dep_name)
-
- if constraints:
- constraints_file = write_new_constraints_file(
- constraints, found_sibling_packages)
-
- for sibling_package in found_sibling_packages:
- log.append("Uninstalling {name}".format(name=sibling_package))
- uninstall_output = subprocess.check_output(
- [tox_python, '-m',
- 'pip', 'uninstall', '-y', sibling_package],
- stderr=subprocess.STDOUT)
- log.extend(uninstall_output.decode('utf-8').split('\n'))
-
- args = [tox_python, '-m', 'pip', 'install']
- if constraints:
- args.extend(['-c', constraints_file])
-
- pkg_root = _get_package_root(sibling_package,
- sibling_python_packages)
- log.append(
- "Installing {name} from {root} for deps".format(
- name=sibling_package,
- root=pkg_root))
- args.append(pkg_root)
-
- install_output = subprocess.check_output(args)
- log.extend(install_output.decode('utf-8').split('\n'))
-
- for sibling_package in found_sibling_packages:
- pkg_root = _get_package_root(sibling_package,
- sibling_python_packages)
- log.append(
- "Installing {name} from {root}".format(
- name=sibling_package,
- root=pkg_root))
-
- install_output = subprocess.check_output(
- [tox_python, '-m', 'pip', 'install', '--no-deps',
- pkg_root])
- log.extend(install_output.decode('utf-8').split('\n'))
- except Exception as e:
- tb = traceback.format_exc()
- log.append(str(e))
- log.append(tb)
- module.fail_json(msg=str(e), log="\n".join(log))
- finally:
- log_text = "\n".join(log)
- module.append_to_file(log_file, log_text)
+ for testenv in envlist:
+ testenv_config = tox_config['testenv:{}'.format(testenv)]
+ envdir = testenv_config['envdir']
+ envlogdir = testenv_config['envlogdir']
+ try:
+ # Write a log file into the .tox dir so that it'll get picked up
+ # Name it with testenv as a prefix so that fetch-tox-output
+ # will properly get it in a multi-env scenario
+ log_file = '{envlogdir}/{testenv}-siblings.txt'.format(
+ envlogdir=envlogdir, testenv=testenv)
+ changed = changed or install_siblings(envdir,
+ projects,
+ package_name,
+ constraints)
+ except Exception as e:
+ tb = traceback.format_exc()
+ log.append(str(e))
+ log.append(tb)
+ module.fail_json(msg=str(e), log="\n".join(log))
+ finally:
+ log_text = "\n".join(log)
+ module.append_to_file(log_file, log_text)
module.exit_json(changed=changed, msg=log_text)
diff --git a/roles/tox/tasks/main.yaml b/roles/tox/tasks/main.yaml
index e14741ba0..fee325608 100644
--- a/roles/tox/tasks/main.yaml
+++ b/roles/tox/tasks/main.yaml
@@ -1,8 +1,3 @@
-- name: Require tox_envlist variable
- fail:
- msg: tox_envlist is required for this role
- when: tox_envlist is not defined
-
- name: Check to see if the constraints file exists
stat:
path: "{{ tox_constraints_file }}"
@@ -25,6 +20,49 @@
UPPER_CONSTRAINTS_FILE: "{{ tox_constraints_file }}"
when: tox_constraints_file is defined
+# Tox siblings cannot take 'ALL' and tox_parse_output expects
+# an envlist to be supplied so always set _tox_envlist equal to
+# the list of testenvs we're going to run
+- name: Set _tox_envlist from supplied tox_envlist
+ set_fact:
+ _tox_envlist: "{{ tox_envlist }}"
+ when:
+ - tox_envlist is defined and tox_envlist
+ - tox_envlist != "ALL"
+
+- name: Get tox default envlist
+ command: "{{ tox_executable }} -l"
+ args:
+ chdir: "{{ zuul_work_dir }}"
+ register: _tox_default_envlist
+ when: tox_envlist is not defined or not tox_envlist
+
+- name: Set tox envlist fact
+ set_fact:
+ _tox_envlist: "{{ _tox_default_envlist.stdout_lines | join(',') }}"
+ when:
+ - _tox_default_envlist is defined
+ - _tox_default_envlist.stdout_lines is defined
+
+- name: Get all tox testenvs
+ command: "{{ tox_executable }} -a"
+ args:
+ chdir: "{{ zuul_work_dir }}"
+ register: _tox_all_testenvs
+ when: tox_envlist is defined and tox_envlist == 'ALL'
+
+- name: Set tox envlist fact
+ set_fact:
+ _tox_envlist: "{{ _tox_all_testenvs.stdout_lines | join(',') }}"
+ when:
+ - _tox_all_testenvs is defined
+ - _tox_all_testenvs.stdout_lines is defined
+
+- name: Fail if tox_envlist is empty
+ fail:
+ msg: "No envlist configured in zuul or tox.ini"
+ when: _tox_envlist is not defined
+
- name: Install tox siblings
include: siblings.yaml
when: tox_install_siblings
@@ -33,9 +71,7 @@
debug:
msg: >-
{{ tox_executable }}
- {% if tox_envlist is defined and tox_envlist %}
- -e{{ tox_envlist }}
- {% endif %}
+ -e{{ _tox_envlist }}
{{ tox_extra_args }}
- block:
@@ -45,9 +81,7 @@
environment: "{{ tox_environment|combine(tox_constraints_env|default({})) }}"
command: >-
{{ tox_executable }}
- {% if tox_envlist is defined and tox_envlist %}
- -e{{ tox_envlist }}
- {% endif %}
+ -e{{ _tox_envlist }}
{{ tox_extra_args }}
register: tox_output
@@ -57,7 +91,7 @@
- name: Look for output
tox_parse_output:
tox_output: '{{ tox_output.stdout }}'
- tox_envlist: '{{ tox_envlist }}'
+ tox_envlist: '{{ _tox_envlist }}'
workdir: '{{ zuul_work_dir }}'
when: tox_inline_comments
register: file_comments
diff --git a/roles/tox/tasks/siblings.yaml b/roles/tox/tasks/siblings.yaml
index 77c650172..65c21bc63 100644
--- a/roles/tox/tasks/siblings.yaml
+++ b/roles/tox/tasks/siblings.yaml
@@ -1,14 +1,35 @@
# Install sibling with tox so we can replace them later
- name: Run tox without tests
- command: "{{ tox_executable }} --notest -e{{ tox_envlist }}"
+ command: >-
+ {{ tox_executable }}
+ --notest
+ -e{{ _tox_envlist }}
args:
chdir: "{{ zuul_work_dir }}"
environment: "{{ tox_environment|combine(tox_constraints_env|default({})) }}"
-# TODO(mordred) handle tox_envlist being a list
+# This is needed since python < 3.2 can't parse ini files from strings
+- name: Create a tempfile to save tox showconfig
+ tempfile:
+ register: _tox_show_config_tempfile
+
+# py27, py35..py38 etc are generated on the fly if not
+# explicitly added to tox.ini, so force tox to generate
+# the config for the testenvs we're using.
+- name: Get tox envlist config
+ shell: "{{ tox_executable }} --showconfig -e {{ _tox_envlist }} > {{ _tox_show_config_tempfile.path }}"
+ args:
+ chdir: "{{ zuul_work_dir }}"
+
- name: Install any sibling python packages
tox_install_sibling_packages:
- tox_envlist: "{{ tox_envlist }}"
+ tox_envlist: "{{ _tox_envlist }}"
+ tox_show_config: "{{ _tox_show_config_tempfile.path }}"
tox_constraints_file: "{{ tox_constraints_file | default(omit) }}"
project_dir: "{{ zuul_work_dir }}"
projects: "{{ zuul.projects.values() | selectattr('required') | list }}"
+
+- name: Remove tempfile
+ file:
+ state: absent
+ path: "{{ _tox_show_config_tempfile.path }}"
diff --git a/test-playbooks/python/tox.ini b/test-playbooks/python/tox.ini
new file mode 100644
index 000000000..ac5f72869
--- /dev/null
+++ b/test-playbooks/python/tox.ini
@@ -0,0 +1,12 @@
+[tox]
+envlist = linters
+skipsdist = true
+
+[testenv]
+whitelist_externals = sh
+
+[testenv:linters]
+commands = sh -c "echo linters >> {posargs}"
+
+[testenv:non-default]
+commands = sh -c "echo non-default >> {posargs}"
diff --git a/test-playbooks/python/tox.yaml b/test-playbooks/python/tox.yaml
new file mode 100644
index 000000000..0658db446
--- /dev/null
+++ b/test-playbooks/python/tox.yaml
@@ -0,0 +1,78 @@
+- hosts: all
+ tasks:
+ - name: Run bindep
+ include_role:
+ name: bindep
+ - name: Run tox with constraints
+ include_role:
+ name: tox
+ vars:
+ tox_envlist: docs
+ tox_constraints_file: '{{ zuul.project.src_dir }}/zuul-tests.d/test-constraints.txt'
+
+ - name: Run tox with multiple testenvs
+ include_role:
+ name: tox
+ vars:
+ tox_envlist: docs,linters
+ tox_environment:
+ ANSIBLE_ROLES_PATH: "{{ ansible_user_dir }}/{{ zuul.project.src_dir }}/roles"
+ ANSIBLE_LIBRARY: "{{ ansible_user_dir }}/{{ zuul.project.src_dir }}/tests/fake-ansible"
+
+ - name: Create tempfile to verify testenvs ran
+ tempfile:
+ register: default_tempfile
+
+ - block:
+ - name: Run tox with empty envlist
+ include_role:
+ name: tox
+ vars:
+ zuul_work_dir: "{{ zuul.project.src_dir }}/test-playbooks/python/"
+ tox_extra_args: "{{ default_tempfile.path }}"
+ tox_install_siblings: false
+ tox_envlist: ''
+ tox_environment:
+ ANSIBLE_ROLES_PATH: "{{ ansible_user_dir }}/{{ zuul.project.src_dir }}/roles"
+ ANSIBLE_LIBRARY: "{{ ansible_user_dir }}/{{ zuul.project.src_dir }}/tests/fake-ansible"
+
+ - name: Make sure magic lines are present
+ lineinfile:
+ path: "{{ default_tempfile.path }}"
+ line: linters
+ check_mode: true
+ register: default_status
+ failed_when: default_status is changed
+ always:
+ - name: Remove tempfile
+ file:
+ state: absent
+ path: "{{ default_tempfile.path }}"
+
+ - block:
+ - name: Create tempfile to verify testenvs ran
+ tempfile:
+ register: ALL_tempfile
+
+ - name: Run tox with ALL
+ include_role:
+ name: tox
+ vars:
+ zuul_work_dir: "{{ zuul.project.src_dir }}/test-playbooks/python/"
+ tox_install_siblings: false
+ tox_extra_args: "{{ ALL_tempfile.path }}"
+ tox_envlist: 'ALL'
+ tox_environment:
+ ANSIBLE_ROLES_PATH: "{{ ansible_user_dir }}/{{ zuul.project.src_dir }}/roles"
+ ANSIBLE_LIBRARY: "{{ ansible_user_dir }}/{{ zuul.project.src_dir }}/tests/fake-ansible"
+ always:
+ - name: Make sure magic lines are present
+ loop:
+ - linters
+ - non-default
+ lineinfile:
+ path: "{{ ALL_tempfile.path }}"
+ line: "{{ item }}"
+ check_mode: true
+ register: ALL_status
+ failed_when: ALL_status is changed
diff --git a/test-playbooks/tox-siblings.yaml b/test-playbooks/tox-siblings.yaml
deleted file mode 100644
index 02a5acff7..000000000
--- a/test-playbooks/tox-siblings.yaml
+++ /dev/null
@@ -1,14 +0,0 @@
-- hosts: all
- tasks:
- - name: Run bindep
- include_role:
- name: bindep
- - name: Run tox with constraints
- include_role:
- name: tox
- vars:
- tox_envlist: docs
- tox_constraints_file: '{{ zuul.project.src_dir }}/zuul-tests.d/test-constraints.txt'
- tox_environment:
- ANSIBLE_ROLES_PATH: "{{ ansible_user_dir }}/{{ zuul.project.src_dir }}/roles"
- ANSIBLE_LIBRARY: "{{ ansible_user_dir }}/{{ zuul.project.src_dir }}/tests/fake-ansible"
diff --git a/zuul-tests.d/python-jobs.yaml b/zuul-tests.d/python-jobs.yaml
index 961227c3d..36cf00af7 100644
--- a/zuul-tests.d/python-jobs.yaml
+++ b/zuul-tests.d/python-jobs.yaml
@@ -400,13 +400,14 @@
label: centos-8-plain
- job:
- name: zuul-jobs-test-tox-siblings
+ name: zuul-jobs-test-tox
description: Test the tox role's sibling functionality
files:
- roles/tox/.*
- tox.ini
- - test-playbooks/tox-siblings.yaml
- run: test-playbooks/tox-siblings.yaml
+ - test-playbooks/python/tox.yaml
+ - test-playbooks/python/tox.ini
+ run: test-playbooks/python/tox.yaml
required-projects:
- zuul/zuul
- zuul/nodepool
@@ -540,7 +541,7 @@
- zuul-jobs-test-fetch-sphinx-tarball-ubuntu-bionic-plain
- zuul-jobs-test-fetch-sphinx-tarball-ubuntu-xenial-plain
- zuul-jobs-test-fetch-sphinx-tarball-centos-8-plain
- - zuul-jobs-test-tox-siblings
+ - zuul-jobs-test-tox
- zuul-jobs-test-fetch-tox-output
- zuul-jobs-test-fetch-tox-output-synchronize
- zuul-jobs-test-fetch-subunit-output
@@ -585,7 +586,7 @@
- zuul-jobs-test-fetch-sphinx-tarball-ubuntu-bionic-plain
- zuul-jobs-test-fetch-sphinx-tarball-ubuntu-xenial-plain
- zuul-jobs-test-fetch-sphinx-tarball-centos-8-plain
- - zuul-jobs-test-tox-siblings
+ - zuul-jobs-test-tox
- zuul-jobs-test-fetch-tox-output
- zuul-jobs-test-fetch-tox-output-synchronize
- zuul-jobs-test-fetch-subunit-output