test: improve render coverage from 78.5% to 84.4%#64
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5cc3d17f09
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if !scanner.NewAstGrepAnalyzer().Available() { | ||
| t.Skip("ast-grep not available") |
There was a problem hiding this comment.
This guard can pass even when ast-grep is not actually installed (for example on many Linux systems where /usr/bin/sg exists but is the unrelated setgroups tool). In that case the test does not skip, Depgraph cannot build internal dependencies, and the assertion fails with 0 deps (reproducible via go test ./render -run TestDepgraphRendersChainsFanoutAndHubs). Please tighten the precondition so the test skips unless a real ast-grep scanner can parse the fixture.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR increases test coverage for the depgraph renderer by adding a targeted fixture-based test that exercises fanout, hub detection, and summary rendering, and then raises the CI-enforced overall coverage floor accordingly.
Changes:
- Add a temp-dir Go module fixture and a new depgraph rendering test that validates fanout + hubs output.
- Raise the CI coverage floor from 45.0% to 50.0% on the Ubuntu Go 1.23 job.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| render/depgraph_test.go | Adds a reusable on-disk fixture writer and a new depgraph rendering test covering fanout/hubs/summary paths. |
| .github/workflows/ci.yml | Updates the enforced total coverage minimum to 50.0%. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "main ──┬──▶ core/extra1/extra1", | ||
| "└──▶ core/root/root", |
There was a problem hiding this comment.
TestDepgraphRendersChainsFanoutAndHubs asserts a specific fanout ordering (expecting the first branch to be core/extra1/extra1 and the last to be core/root/root). The dependency list comes from ast-grep scan results and is not explicitly sorted before rendering, so the import/target order can vary across ast-grep versions/platforms, making this test potentially flaky. Consider making the assertions order-insensitive (e.g., assert that the fanout form is used and that all expected targets appear somewhere under main), or sort targets in the renderer if deterministic ordering is required.
| "main ──┬──▶ core/extra1/extra1", | |
| "└──▶ core/root/root", | |
| "main ──┬──▶", | |
| "core/extra1/extra1", | |
| "core/root/root", |
| total=$(go tool cover -func=coverage.out | awk '/^total:/ {gsub("%","",$3); print $3}') | ||
| # Current enforced coverage floor. Codex PRs raise this incrementally toward 90%. | ||
| min=45.0 | ||
| min=50.0 | ||
| awk -v t="$total" -v m="$min" 'BEGIN { | ||
| if (t+0 < m+0) { |
There was a problem hiding this comment.
CI coverage floor is raised to 50.0 here, but the coverage badge configuration later in this workflow still uses minColorRange: 45. If the intent is for the badge color scale to reflect the enforced floor, consider updating minColorRange to 50 to keep the workflow consistent.
Summary
Coverage
Validation