ci: ansible-lint requires dependencies to be installed [citest_skip] - #80
Merged
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 guide (collapsed on small PRs)Reviewer's GuideUpdates CI workflows to use tox-lsr 3.18.1 and refactors ansible-lint and ansible-test jobs so that the collection build step is separated from the lint/test execution, relying on tox-lsr to install collection requirements correctly. Sequence diagram for updated ansible-lint CI workflowsequenceDiagram
actor Developer
participant GitHubActions
participant Runner
participant tox_lsr_3_18_1 as tox_lsr_3_18_1
participant tox
participant AnsibleCollection as ansible_collection
participant AnsibleLint as ansible_lint
Developer->>GitHubActions: push branch / open PR
GitHubActions->>Runner: start ansible-lint workflow
Runner->>Runner: checkout repository
Runner->>Runner: setup Python matrix version
Runner->>tox_lsr_3_18_1: pip install tox-lsr 3.18.1
tox_lsr_3_18_1-->>Runner: provides tox environments
Note over Runner,AnsibleCollection: Step 1 - build collection separately
Runner->>tox: tox -e collection
tox->>AnsibleCollection: convert role to collection layout
AnsibleCollection-->>tox: built collection with dependencies from collection-requirements
tox-->>Runner: collection environment ready
Note over Runner,AnsibleLint: Step 2 - run ansible-lint only
Runner->>tox: tox -e ansible-lint-collection
tox->>AnsibleLint: run with LSR_ANSIBLE_LINT_DEP and LSR_ANSIBLE_LINT_ANSIBLE_DEP
AnsibleLint-->>tox: lint results
tox-->>Runner: job status
Runner-->>GitHubActions: report ansible-lint job result
GitHubActions-->>Developer: CI status on PR
Flow diagram for refactored ansible-lint job stepsflowchart TD
A[Start ansible-lint job] --> B[Checkout repository]
B --> C[Set up Python using matrix version]
C --> D[Install tox-lsr 3.18.1 with pip]
D --> E[Run tox -e collection]
E --> F[Role converted to collection format
and dependencies from collection requirements installed]
F --> G[Set LSR_ANSIBLE_LINT_DEP and
LSR_ANSIBLE_LINT_ANSIBLE_DEP environment variables]
G --> H[Run tox -e ansible-lint-collection
with matrix basepython]
H --> I[ansible-lint runs against built collection]
I --> J[Report job status]
J --> K[End ansible-lint job]
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:
- The tox-lsr version string is duplicated across multiple workflows; consider defining it in a single reusable place (e.g., a workflow env var or a shared action) to make future version bumps less error-prone.
- Now that
tox -e collectionis run in separate steps for ansible-lint and ansible-test, verify whether the collection build artifacts can be reused between jobs (e.g., viaactions/cacheor artifacts) to avoid rebuilding the collection for each job/matrix entry.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The tox-lsr version string is duplicated across multiple workflows; consider defining it in a single reusable place (e.g., a workflow env var or a shared action) to make future version bumps less error-prone.
- Now that `tox -e collection` is run in separate steps for ansible-lint and ansible-test, verify whether the collection build artifacts can be reused between jobs (e.g., via `actions/cache` or artifacts) to avoid rebuilding the collection for each job/matrix entry.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 CI workflows to use the latest tox-lsr release and separate collection conversion from ansible linting and testing steps.
CI: