ci(integration-tests): Add workflow for integration tests.#2334
ci(integration-tests): Add workflow for integration tests.#2334quinntaylormitchell wants to merge 12 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughA new GitHub Actions workflow file ( ChangesIntegration Tests CI Workflow
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/clp-integration-tests.yaml:
- Line 6: The path-ignore pattern `.github/*` uses a single wildcard which only
matches immediate children of the `.github/` directory and will not match nested
subdirectories like `.github/workflows/`. To ensure consistency with the
recursive matching pattern (`**`) used in lines 7-8 and to properly ignore all
changes under the entire `.github/` directory, update the pattern from
`.github/*` to `.github/**`.
- Around line 29-31: The actions/checkout step in the workflow defaults to
persist-credentials: true, which stores the GitHub token in the git config and
could expose it if untrusted code is executed during the task tests:integration
command. Add persist-credentials: false to the with section of the
actions/checkout action to disable credential persistence, unless subsequent
workflow steps explicitly require authenticated git operations.
- Around line 23-43: The workflow currently lacks an explicit permissions block,
which means it inherits default GitHub Actions permissions that may be broader
than necessary. Add a top-level permissions block after the concurrency block
(if present) to define minimal explicit permissions required for the integration
tests workflow. Set all default permissions to false and only enable specific
permissions that the integration tests genuinely require (such as contents: read
for checking out code). If the integration tests need additional access like
pulling from GitHub Container Registry, add packages: read accordingly. This
follows the principle of least privilege and reduces security risk if the
workflow or its dependencies are compromised.
- Line 24: Add a descriptive `name` field to the `integration-tests` job
definition in the workflow file. After the job ID `integration-tests:`, add a
`name` property with a clear, human-readable description of what the job does.
This improves visibility and readability in the GitHub Actions UI when viewing
workflow runs.
- Around line 37-39: The uv installation action in the install-uv step specifies
the version as "0.8" in shorthand format, but the uv package manager requires
the complete semantic versioning format with the patch version included. Change
the version parameter value from "0.8" to "0.8.0" to match the canonical semver
format that uv uses for official releases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c447b93e-4b15-4cc2-93bc-6464d321aa04
📒 Files selected for processing (1)
.github/workflows/clp-integration-tests.yaml
| on: | ||
| pull_request: | ||
| paths-ignore: &ignored_paths | ||
| - ".github/*" |
There was a problem hiding this comment.
Inconsistent path-ignore pattern may not match subdirectories.
The pattern .github/* uses a single wildcard that only matches immediate children of .github/, not subdirectories like .github/workflows/. Lines 7-8 use ** for recursive matching. If the intent is to ignore all changes under .github/ (as suggested by the summary mentioning "ignoring .github/**"), use .github/** for consistency.
📝 Proposed fix
- - ".github/*"
+ - ".github/**"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - ".github/*" | |
| - ".github/**" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/clp-integration-tests.yaml at line 6, The path-ignore
pattern `.github/*` uses a single wildcard which only matches immediate children
of the `.github/` directory and will not match nested subdirectories like
`.github/workflows/`. To ensure consistency with the recursive matching pattern
(`**`) used in lines 7-8 and to properly ignore all changes under the entire
`.github/` directory, update the pattern from `.github/*` to `.github/**`.
| jobs: | ||
| integration-tests: | ||
|
|
||
| # Runs only on Ubuntu Noble (24.04) because the integration test suite can't run on macOS. | ||
| runs-on: "ubuntu-24.04" | ||
| steps: | ||
| - uses: "actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd" # v6.0.2 | ||
| with: | ||
| submodules: "recursive" | ||
|
|
||
| - uses: "./tools/yscope-dev-utils/exports/github/actions/install-go-task" | ||
| with: | ||
| version: "3.48.0" | ||
|
|
||
| - uses: "./tools/yscope-dev-utils/exports/github/actions/install-uv" | ||
| with: | ||
| version: "0.8" | ||
|
|
||
| - name: "Run integration tests" | ||
| shell: "bash" | ||
| run: "task tests:integration" |
There was a problem hiding this comment.
Define explicit minimal permissions for the workflow.
No explicit permissions block means the workflow inherits default permissions, which may grant broader access than required. Integration tests typically need minimal permissions. Following the principle of least privilege reduces the security risk if the workflow or its dependencies are compromised.
🔐 Recommended permissions block
Add at the top level after the concurrency block:
concurrency:
group: "${{github.workflow}}-${{github.ref}}"
# Cancel in-progress jobs for efficiency; Exclude the `main` branch.
cancel-in-progress: "${{github.ref != 'refs/heads/main'}}"
+permissions:
+ contents: read
+
jobs:
integration-tests:Adjust permissions if the integration tests require additional access (e.g., packages: read if pulling from GitHub Container Registry).
As per static analysis tool zizmor, which flagged "overly broad permissions (excessive-permissions)".
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 29-31: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[info] 24-24: workflow or action definition without a name (anonymous-definition): this job
(anonymous-definition)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/clp-integration-tests.yaml around lines 23 - 43, The
workflow currently lacks an explicit permissions block, which means it inherits
default GitHub Actions permissions that may be broader than necessary. Add a
top-level permissions block after the concurrency block (if present) to define
minimal explicit permissions required for the integration tests workflow. Set
all default permissions to false and only enable specific permissions that the
integration tests genuinely require (such as contents: read for checking out
code). If the integration tests need additional access like pulling from GitHub
Container Registry, add packages: read accordingly. This follows the principle
of least privilege and reduces security risk if the workflow or its dependencies
are compromised.
Source: Linters/SAST tools
| cancel-in-progress: "${{github.ref != 'refs/heads/main'}}" | ||
|
|
||
| jobs: | ||
| integration-tests: |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Consider adding a descriptive name field to the job.
The job has an ID (integration-tests) but no name field. Adding a name improves readability in the GitHub Actions UI.
📝 Proposed addition
jobs:
integration-tests:
+ name: "Integration Tests"
# Runs only on Ubuntu Noble (24.04) because the integration test suite can't run on macOS.
runs-on: "ubuntu-24.04"As per static analysis tool zizmor, which noted "workflow or action definition without a name (anonymous-definition)".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| integration-tests: | |
| integration-tests: | |
| name: "Integration Tests" |
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 24-44: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
[info] 24-24: workflow or action definition without a name (anonymous-definition): this job
(anonymous-definition)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/clp-integration-tests.yaml at line 24, Add a descriptive
`name` field to the `integration-tests` job definition in the workflow file.
After the job ID `integration-tests:`, add a `name` property with a clear,
human-readable description of what the job does. This improves visibility and
readability in the GitHub Actions UI when viewing workflow runs.
Source: Linters/SAST tools
| - uses: "actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd" # v6.0.2 | ||
| with: | ||
| submodules: "recursive" |
There was a problem hiding this comment.
Disable credential persistence to prevent token leakage.
The checkout action defaults to persist-credentials: true, which stores the GitHub token in the local git config. If the task tests:integration command builds or executes untrusted code, or if dependencies are compromised, the token could be leaked. Set persist-credentials: false unless git operations are required in subsequent steps.
🔐 Proposed fix
- uses: "actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd" # v6.0.2
with:
submodules: "recursive"
+ persist-credentials: falseAs per static analysis tool zizmor, which flagged "credential persistence through GitHub Actions artefacts (artipacked)".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - uses: "actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd" # v6.0.2 | |
| with: | |
| submodules: "recursive" | |
| - uses: "actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd" # v6.0.2 | |
| with: | |
| submodules: "recursive" | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 29-31: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/clp-integration-tests.yaml around lines 29 - 31, The
actions/checkout step in the workflow defaults to persist-credentials: true,
which stores the GitHub token in the git config and could expose it if untrusted
code is executed during the task tests:integration command. Add
persist-credentials: false to the with section of the actions/checkout action to
disable credential persistence, unless subsequent workflow steps explicitly
require authenticated git operations.
Source: Linters/SAST tools
| - uses: "./tools/yscope-dev-utils/exports/github/actions/install-uv" | ||
| with: | ||
| version: "0.8" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check the install-uv action's expected version format by examining its inputs definition.
# Check the action.yml for the install-uv action to see version format requirements
fd -t f 'action.ya?ml' tools/yscope-dev-utils/exports/github/actions/install-uv/ --exec cat {}Repository: y-scope/clp
Length of output: 200
🏁 Script executed:
fd -t f 'action.ya?ml'Repository: y-scope/clp
Length of output: 37
🏁 Script executed:
fd -t d 'install-uv'Repository: y-scope/clp
Length of output: 37
🏁 Script executed:
rg -t yaml 'install-uv' --max-count=10Repository: y-scope/clp
Length of output: 37
🏁 Script executed:
cat -n .github/workflows/clp-integration-tests.yamlRepository: y-scope/clp
Length of output: 1512
🏁 Script executed:
git ls-files | grep -i 'yscope\|install-uv'Repository: y-scope/clp
Length of output: 553
🏁 Script executed:
find . -name 'action.yml' -o -name 'action.yaml' 2>/dev/null | head -20Repository: y-scope/clp
Length of output: 208
🏁 Script executed:
cat .gitmodules 2>/dev/null || echo "No .gitmodules file"Repository: y-scope/clp
Length of output: 178
🏁 Script executed:
ls -la tools/yscope-dev-utils/ 2>/dev/null | head -20Repository: y-scope/clp
Length of output: 164
🏁 Script executed:
find tools/yscope-dev-utils -name 'action.ya?ml' 2>/dev/nullRepository: y-scope/clp
Length of output: 37
🌐 Web query:
yscope-dev-utils install-uv GitHub action version format
💡 Result:
There is no GitHub Action officially named yscope-dev-utils/install-uv. The primary action for setting up uv in GitHub Actions workflows is astral-sh/setup-uv [1][2]. If you are looking to specify the version format for the astral-sh/setup-uv action, the version input accepts the following formats [3]: - Specific Version: A concrete semver string (e.g., "0.4.4") [3]. - Latest: The string "latest" (default behavior if no version is specified) [3]. - Version Ranges: Semver ranges (e.g., ">=0.4.0") [3]. - Configuration Files: You can use the version-file input to point to a pyproject.toml or uv.toml file, and the action will resolve the required version from those files [3]. If you are using the y-scope/yscope-dev-utils repository [4], it appears to be a collection of C++ linting configurations, scripts, and internal tools, rather than a provider of a uv installation action [4][5]. If you have encountered a workflow referring to yscope-dev-utils, it may be a private or custom reference unique to your project's internal tooling [4].
Citations:
- 1: https://github.com/astral-sh/setup-uv?tab=readme-ov-file
- 2: https://github.com/marketplace/actions/astral-sh-setup-uv
- 3: https://github.com/astral-sh/setup-uv/tree/eb1897b8dc4b5d5bfe39a428a8f2304605e0983c
- 4: https://github.com/y-scope/yscope-dev-utils
- 5: https://github.com/y-scope/yscope-dev-utils/blob/main/docs/lint-tools-cpp.md
🌐 Web query:
uv package manager version format "0.8" valid
💡 Result:
The version string "0.8" is generally not a valid, canonical version format for a release of the uv package manager itself, nor is it typically used as a complete version identifier in Python packaging contexts. Official releases of the uv package manager follow a major.minor.patch versioning scheme (e.g., 0.8.0) [1][2][3]. Versions like "0.8.0" are valid and were officially released [1][2]. In Python packaging more broadly—which uv adheres to [4]—version specifiers and version identifiers must follow PEP 440 standards [4]. A simple "0.8" string is often interpreted as a truncated version identifier. While some systems or tools might permit such shorthand during input, it is generally normalized to a canonical form like "0.8.0" for internal operations, comparison, and metadata [4]. If you are referring to the uv package manager's own versioning policy: - uv uses a custom scheme where the minor version is bumped for breaking changes and the patch version is bumped for non-breaking changes [5][6]. - "0.8" is not used as a full release identifier; the project explicitly uses "0.8.0", "0.8.1", etc. [2] If you are using "0.8" in a dependency constraint (e.g., in a pyproject.toml file), it may be accepted by some resolvers as a prefix or shorthand, but using the explicit canonical format (e.g., "==0.8.0" or ">=0.8.0") is recommended to ensure compatibility and avoid ambiguity [7].
Citations:
- 1: https://github.com/astral-sh/uv/releases/tag/0.8.0
- 2: https://pypi.org/project/uv/
- 3: https://pypi.org/project/uv/0.8.8/
- 4: uv changes my package version when doing "uv build --sdist" or "uv lock" astral-sh/uv#7875
- 5: https://github.com/astral-sh/uv/blob/main/docs/reference/policies/versioning.md
- 6: https://docs.astral.sh/uv/reference/policies/versioning/
- 7: https://astral-sh-uv.mintlify.app/concepts/dependencies
🌐 Web query:
uv Python installer version format requirements
💡 Result:
The uv Python installer supports several flexible formats for requesting Python versions, which can be used in commands (such as uv python install or --python), configuration files (like.python-version), or inline script metadata [1][2][3]. Supported request formats include: Version numbers: Simple strings specifying the version (e.g., 3, 3.12, 3.12.3) [1][2]. Version constraints: Standard PEP 440 specifiers (e.g., >=3.12,<3.13) [1][2][4]. Specific build variants: Versions with suffixes (e.g., 3.13t, 3.12.0d) [1][2]. Build metadata/freethreaded: Versions with + markers (e.g., 3.13+freethreaded, 3.12.0+debug, 3.14+gil) [1][2]. Implementation identifiers: Names (e.g., cpython or cp) [1][2]. Implementation with version: Combinations using @ (e.g., cpython@3.12) or shorthand prefixes (e.g., cpython3.12, cp312) [1][2]. Implementation with constraints: Combinations using constraints (e.g., cpython>=3.12,<3.13) [1][2]. Full platform identifiers: Specific build strings (e.g., cpython-3.12.3-macos-aarch64-none) [1][2]. Additionally, uv supports requesting specific local system interpreters by providing a file path (e.g., /opt/homebrew/bin/python3), an environment name (e.g., mypython3), or a directory path (e.g., /some/environment/) [1][2]. When using these formats, uv respects project-level configurations like.python-version files (which can contain any of the supported formats) and requires-python fields in pyproject.toml [1][2][5]. While most formats are broadly supported, requests involving specific local file paths are generally restricted to flags (e.g., --python) and are not used for general installation requests [1][2]. Using a simple version number is recommended for better interoperability with other tools [1][2].
Citations:
- 1: https://docs.astral.sh/uv/concepts/python-versions/index.md
- 2: https://docs.astral.sh/uv/concepts/python-versions/
- 3: https://github.com/astral-sh/uv/blob/93fc6f04/crates/uv/tests/it/help.rs
- 4: https://deepwiki.com/astral-sh/uv/3.2-requirements-and-version-parsing
- 5: https://docs.astral.sh/uv/guides/scripts/index.md
Correct the uv version format to include patch version.
The version string should be "0.8.0" instead of "0.8". The uv package manager uses major.minor.patch versioning (e.g., official releases are 0.8.0, 0.8.1, etc.), not major.minor only. While some installation actions may accept and normalize shorthand versions, using the canonical semver format "0.8.0" is the recommended practice.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/clp-integration-tests.yaml around lines 37 - 39, The uv
installation action in the install-uv step specifies the version as "0.8" in
shorthand format, but the uv package manager requires the complete semantic
versioning format with the patch version included. Change the version parameter
value from "0.8" to "0.8.0" to match the canonical semver format that uv uses
for official releases.
1 participant
Description
This PR adds an automated workflow for integration tests. The workflow runs
task tests:integration.Checklist
breaking change.
Validation performed
Summary by CodeRabbit