test: improve render coverage from 78.5% to 81.1%#63
Conversation
There was a problem hiding this comment.
Pull request overview
This PR increases render-related test coverage by adding new deterministic tests around skyline animation behavior and depgraph rendering, and tightens CI enforcement by raising the minimum required overall coverage.
Changes:
- Added unit tests for skyline tick/init and phase-2 animation transitions (shooting star, completion).
- Added a depgraph test intended to cover long external dependency line wrapping behavior.
- Increased the CI coverage floor from 45.0% to 50.0%.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
render/skyline_test.go |
Adds tests for tickCmd, animationModel.Init, and phase-2 Update transitions to improve render coverage. |
render/depgraph_test.go |
Adds a test targeting long external dependency output (wrapping/formatting) to improve depgraph coverage. |
.github/workflows/ci.yml |
Raises the enforced minimum coverage threshold to 50.0% in CI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| msg := cmd() | ||
| if _, ok := msg.(tickMsg); !ok { | ||
| t.Fatalf("expected tickMsg, got %T", msg) | ||
| } |
There was a problem hiding this comment.
TestSkylineTickCmd invokes the tea.Tick command (cmd()), which will sleep for the configured 60ms tick interval. This introduces real time delays into the unit test suite and can slow/flakify CI; consider only asserting cmd is non-nil, or refactor tickCmd to allow injecting a zero/short duration in tests so the command can be executed without waiting.
| msg := cmd() | |
| if _, ok := msg.(tickMsg); !ok { | |
| t.Fatalf("expected tickMsg, got %T", msg) | |
| } |
| } | ||
| if !strings.Contains(output, "super-long-package-alpha") || !strings.Contains(output, "super-long-package-epsilon") { | ||
| t.Fatalf("expected long dependency names to be present, got:\n%s", output) | ||
| } |
There was a problem hiding this comment.
TestDepgraphWrapsLongDependencyLines doesn't currently assert any wrapping behavior (e.g., a continuation line or indentation) — it only checks that dependency names are present, which would also pass if wrapping regressed. To validate wrapping, assert that the output contains at least one wrapped/continued line for the Go dependency section (for example, multiple boxed lines for the Go deps and/or the expected continuation indentation).
| } | |
| } | |
| // Ensure that the Go dependency section is actually wrapped across multiple lines, | |
| // not rendered as a single overly long line. | |
| lines := strings.Split(output, "\n") | |
| wrappedLines := 0 | |
| for _, line := range lines { | |
| if strings.Contains(line, "super-long-package") { | |
| wrappedLines++ | |
| } | |
| } | |
| if wrappedLines < 2 { | |
| t.Fatalf("expected Go dependencies to be wrapped across multiple lines (found %d line), got:\n%s", wrappedLines, output) | |
| } |
| 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.
The enforced coverage floor is raised to 50.0 here, but the badge color range configuration later in this workflow still uses minColorRange: 45. If the badge is meant to reflect the enforced floor, consider updating minColorRange to 50 (or sourcing it from the same variable) to avoid showing a green-ish badge while the job would fail below 50%.
Summary
Coverage
Validation