Skip to content

chore: enable parallel tests, add uv caching, validate builds on all platforms [ADS-739] - #406

Merged
hemang-snyk merged 2 commits into
mainfrom
github-workflow-updy2
Jul 21, 2026
Merged

chore: enable parallel tests, add uv caching, validate builds on all platforms [ADS-739]#406
hemang-snyk merged 2 commits into
mainfrom
github-workflow-updy2

Conversation

@hemang-snyk

@hemang-snyk hemang-snyk commented Jul 20, 2026

Copy link
Copy Markdown
Contributor
  • Enables parallel testing across all platforms by removing max-parallel: 1
  • Adds uv dependency caching subsequent runs on the same branch run faster.
  • Builds both the wheel and the binary on every platform for sanity (previously ubuntu-only)
  • Uploads the built artifacts (binary + wheel) only from the main branch and not on PRs

@hemang-snyk
hemang-snyk requested a review from a team as a code owner July 20, 2026 14:48
@qodo-merge-etso

Copy link
Copy Markdown

PR Summary by Qodo

CI: parallelize tests, cache uv deps, validate builds on all OSes

⚙️ Configuration changes 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Remove test job serialization so matrix runs in parallel.
• Enable uv dependency caching and run commands via uv run --frozen.
• Build wheel and binary on every supported OS to validate cross-platform builds.
Diagram

graph TD
  A["GitHub Actions: Tests"] --> B["pre-commit job"] --> C["setup-python 3.12"] --> D["pre-commit hooks"]
  A --> E["test matrix (5 OS)"] --> F(["setup-uv"]) --> G["uv run make ci"] --> H["uv run make build"] --> I["uv run make binary"]
  F --> J[("uv cache")]
  subgraph Legend
    direction LR
    _job["Job/Step"] ~~~ _tool(["Tool"]) ~~~ _cache[("Cache")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Split build validation into a separate job/workflow
  • ➕ Keeps the core test signal fast while still validating cross-platform builds
  • ➕ Allows different triggers (e.g., nightly, release, main-only) for heavier builds
  • ➖ More workflow complexity and more places to maintain environment setup
  • ➖ Build failures may be noticed later depending on trigger strategy
2. Only build wheel/binary on a subset of platforms per PR
  • ➕ Reduces CI time/cost while still catching many build issues early
  • ➕ Can prioritize the riskiest/most-used platforms
  • ➖ Misses platform-specific breakages until later
  • ➖ Requires maintaining a policy for which platforms are “required” vs “best effort”
3. Upload artifacts conditionally (e.g., only on main or tags)
  • ➕ Avoids extra artifact storage/retention concerns for PRs
  • ➕ Still validates build steps on all platforms
  • ➖ Doesn’t improve runtime if builds still run everywhere
  • ➖ Less convenient for reviewers who want to download PR artifacts

Recommendation: The PR’s approach is solid for maximizing confidence: running the full matrix in parallel and building wheel+binary everywhere catches platform-specific breakages early. If CI runtime becomes an issue, the best next step would be splitting build validation into a separate job/workflow with tailored triggers, while keeping the same uv run --frozen execution model and caching.

Files changed (1) +19 / -26

Other (1) +19 / -26
tests.ymlParallelize CI matrix, cache uv, and build artifacts on all OSes +19/-26

Parallelize CI matrix, cache uv, and build artifacts on all OSes

• Updates GitHub Actions dependencies, pins Python 3.12 for the pre-commit job, and removes the test job’s max-parallel constraint. Switches to 'uv run --frozen' with uv caching (keyed by uv.lock) and runs wheel+binary builds on every matrix platform instead of ubuntu-only.

.github/workflows/tests.yml

@qodo-merge-etso

qodo-merge-etso Bot commented Jul 20, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 8 rules

Grey Divider


Action required

1. Windows wheel build breaks 🐞 Bug ☼ Reliability
Description
The workflow now runs make build on windows-latest, but make build depends on clean which
uses POSIX rm -rf commands and is not compatible with the default Windows runner shell/tooling.
This can cause the new Windows wheel-validation step to fail before uv build executes.
Code

.github/workflows/tests.yml[R49-50]

+      - name: Build wheel
+        run: uv run --frozen make build
Relevance

⭐⭐⭐ High

Team often fixes Windows compatibility issues (Windows path/test fixes merged in PR #239 and Windows
support in #167).

PR-#239
PR-#167

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The workflow explicitly runs the wheel build on Windows, and the Makefile build target invokes
clean, which uses rm -rf commands that are POSIX-specific.

.github/workflows/tests.yml[21-52]
Makefile[40-46]
Makefile[62-64]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
CI now runs `uv run --frozen make build` on Windows. The Makefile `build` target depends on `clean`, and `clean` uses POSIX `rm -rf`, which is not reliably available/compatible on GitHub Actions Windows runners, causing wheel validation to fail.

### Issue Context
The workflow matrix includes `windows-latest` and unconditionally runs `make build`.

### Fix Focus Areas
- Makefile[40-46]
- Makefile[62-64]
- .github/workflows/tests.yml[34-52]

### Suggested fix approach
- Make `clean` cross-platform:
 - Option A: implement `clean` via Python (portable): `python -c "import shutil; shutil.rmtree('dist', ignore_errors=True)"` etc.
 - Option B: add an OS-conditional branch in the Makefile (e.g., `ifeq ($(OS),Windows_NT)`), using PowerShell `Remove-Item -Recurse -Force` for Windows.
- Alternatively (less ideal if you truly want Windows validation): guard `Build wheel` to skip Windows until Makefile is portable.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. PyInstaller add-data Windows separator ✓ Resolved 🐞 Bug ≡ Correctness
Description
The workflow now validates make binary on windows-latest, but the Makefile passes `--add-data
'src/agent_scan/hooks:agent_scan/hooks' to PyInstaller using a :` separator. On Windows,
PyInstaller expects a different separator, which can break the Windows binary build or produce an
incomplete executable.
Code

.github/workflows/tests.yml[R51-52]

+      - name: Build binary
+        run: uv run --frozen make binary
Relevance

⭐⭐⭐ High

Team prioritizes PyInstaller/binary build correctness (PyInstaller add-data fix in PR #260; Makefile
binary work in #265).

PR-#260
PR-#265

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The workflow now runs binary builds on Windows, and the Makefile’s PyInstaller invocation uses a
hard-coded --add-data argument formatted for Unix-like platforms.

.github/workflows/tests.yml[34-52]
Makefile[49-60]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
CI now runs `make binary` on Windows. The Makefile hard-codes a `--add-data` argument using the Unix-style `SRC:DEST` separator, which is not the Windows format for PyInstaller.

### Issue Context
The workflow matrix includes `windows-latest` and unconditionally runs `make binary`.

### Fix Focus Areas
- Makefile[49-60]
- .github/workflows/tests.yml[34-52]

### Suggested fix approach
- Introduce an OS-aware separator in the Makefile, e.g.:
 - `ifeq ($(OS),Windows_NT) ADD_DATA_SEP := ; else ADD_DATA_SEP := : endif`
 - Use `--add-data "src/agent_scan/hooks$(ADD_DATA_SEP)agent_scan/hooks"`.
- Alternatively, move PyInstaller configuration to a `.spec` file to avoid platform-specific CLI quirks, and call `pyinstaller agent-scan.spec` from `make binary`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Binary built twice per job ✓ Resolved 🐞 Bug ➹ Performance
Description
Each matrix job runs make ci (which already runs $(MAKE) binary) and then runs a separate `make
binary` step, redundantly rebuilding the binary on every platform. This unnecessarily increases CI
time and introduces extra build surface area per run.
Code

.github/workflows/tests.yml[R47-52]

+      - name: Run tests
+        run: uv run --frozen make ci
+      - name: Build wheel
+        run: uv run --frozen make build
+      - name: Build binary
+        run: uv run --frozen make binary
Relevance

⭐⭐ Medium

No accepted history on removing redundant CI steps; similar perf/caching refactors were rejected in
PR #265.

PR-#265

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The workflow runs make ci and then make binary, while the Makefile shows that ci already
invokes binary via $(MAKE) binary.

.github/workflows/tests.yml[47-52]
Makefile[31-34]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The workflow builds the binary twice: once inside `make ci` and again in the dedicated `Build binary` step.

### Issue Context
`make ci` invokes `$(MAKE) binary` before running pytest.

### Fix Focus Areas
- .github/workflows/tests.yml[47-52]
- Makefile[31-34]

### Suggested fix approach
- Option A (simplest): remove the standalone `Build binary` step, since `make ci` already covers it.
- Option B (cleaner separation): refactor Makefile so `ci` only runs tests, and in the workflow do:
 1) `make binary` (once)
 2) `pytest ... --runner=binary` (or add a new make target like `ci-test`)
 3) `make build`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread .github/workflows/tests.yml
Comment thread .github/workflows/tests.yml Outdated
Comment thread .github/workflows/tests.yml Outdated
@hemang-snyk
hemang-snyk force-pushed the github-workflow-updy2 branch 3 times, most recently from c4de36b to 76a6245 Compare July 20, 2026 18:16
@hemang-snyk
hemang-snyk force-pushed the github-workflow-updy2 branch from 76a6245 to 14223c2 Compare July 20, 2026 18:22
@hemang-snyk
hemang-snyk enabled auto-merge July 21, 2026 07:46
@hemang-snyk
hemang-snyk merged commit f52094a into main Jul 21, 2026
10 checks passed
@hemang-snyk hemang-snyk changed the title chore: enable parallel tests, add uv caching, validate builds on all platforms chore: enable parallel tests, add uv caching, validate builds on all platforms [ADS-739] Jul 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants