ci: ansible-lint requires dependencies to be installed [citest_skip]#25
Closed
richm wants to merge 1 commit into
Closed
ci: ansible-lint requires dependencies to be installed [citest_skip]#25richm wants to merge 1 commit into
richm wants to merge 1 commit into
Conversation
ansible-lint requires the dependencies in meta/collection-requirements.yml and tests/collection-requirements.yml to be installed. tox-lsr 3.18.1 will ensure they are installed. Refactor the tests somewhat so that the collection and test steps are separate. Signed-off-by: Rich Megginson <rmeggins@redhat.com>
Reviewer's GuideUpdates CI workflows to use tox-lsr 3.18.1, refactors ansible-lint and ansible-test jobs to run via tox with explicit version matrices for ansible/ansible-lint/python, and adds a reusable Ansible task file to clear facts and run the linux-system-roles.trustee_client role with optional failure suppression. Flow diagram for updated ansible-test and ansible-managed-var-comment tox-lsr usageflowchart LR
subgraph GitHubActions
wf_ansible_test["Workflow ansible-test.yml"]
wf_managed_var["Workflow ansible-managed-var-comment.yml"]
end
dev["Developer push / PR"] --> wf_ansible_test
dev --> wf_managed_var
subgraph SharedSteps["Shared CI pattern"]
step_update_tools["Update pip, git"]
step_install_tox_lsr["Install tox-lsr 3.18.1"]
end
wf_ansible_test --> step_update_tools
wf_managed_var --> step_update_tools
step_update_tools --> step_install_tox_lsr
subgraph ansible_test_job["ansible-test job"]
run_tox_ansible_test["Run tox-managed ansible-test envs"]
end
subgraph managed_var_job["ansible-managed-var-comment job"]
run_ansible_plugin_scan["Run ansible-plugin-scan via tox-lsr"]
end
step_install_tox_lsr --> run_tox_ansible_test
step_install_tox_lsr --> run_ansible_plugin_scan
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In the ansible-lint and ansible-test workflows, the
actions/setup-pythonstep runs after installingtox/tox-lsr, which means those tools are installed into the runner’s default Python rather than the matrix Python; consider movingsetup-pythonbefore the pip install so tox and plugins are consistently tied to the selected Python version. - The
tox -x ...basepython="python${{ matrix.versions.python }}"usage assumes executables likepython3.12/python3.13exist on PATH, which is not guaranteed byactions/setup-python(it only providespython/python3); it may be more robust to drop thebasepythonoverride and rely on the selected interpreter, or derive the correct executable name dynamically (e.g., via$(python -c ...)).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the ansible-lint and ansible-test workflows, the `actions/setup-python` step runs after installing `tox`/`tox-lsr`, which means those tools are installed into the runner’s default Python rather than the matrix Python; consider moving `setup-python` before the pip install so tox and plugins are consistently tied to the selected Python version.
- The `tox -x ...basepython="python${{ matrix.versions.python }}"` usage assumes executables like `python3.12`/`python3.13` exist on PATH, which is not guaranteed by `actions/setup-python` (it only provides `python`/`python3`); it may be more robust to drop the `basepython` override and rely on the selected interpreter, or derive the correct executable name dynamically (e.g., via `$(python -c ...)`).Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ansible-lint requires the dependencies in meta/collection-requirements.yml
and tests/collection-requirements.yml to be installed. tox-lsr 3.18.1
will ensure they are installed.
Refactor the tests somewhat so that the collection and test steps are separate.
Signed-off-by: Rich Megginson rmeggins@redhat.com
Summary by Sourcery
Update Ansible CI workflows to use newer tox-lsr and delegate linting and testing to tox-based collection and ansible-test environments across multiple Ansible and Python versions.
CI:
Tests: