Skip to content

test: improve render coverage from 78.5% to 86.9% #67

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JordanCoin merged 2 commits into from
Jul 27, 2026
Merged

test: improve render coverage from 78.5% to 86.9% #67

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7c5e468
test: improve render coverage from 78.5% to 86.9%
JordanCoin Apr 2, 2026
cbb5a99
Merge remote-tracking branch 'origin/main' into pr67-fix
JordanCoin Jul 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions render/clone_animation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,18 @@ func TestCloneAnimationBuildFrame(t *testing.T) {
}
}
}

func TestCloneAnimationDemo(t *testing.T) {
var buf bytes.Buffer
a := NewCloneAnimation(&buf, "repo")

a.Demo()

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Replace slow Demo call in unit test

TestCloneAnimationDemo invokes Demo(), which sleeps 80ms for each of 51 frames plus a final 500ms pause, adding about 4.6 seconds to every run of this package even though the assertion only checks final output content. This materially slows local/CI feedback loops and is avoidable by testing Render/buildFrame directly or injecting a mock sleeper/clock.

Useful? React with 👍 / 👎.


out := buf.String()
if !strings.Contains(out, "100%") {
t.Fatalf("expected demo output to reach 100%%, got %q", out)
}
if !strings.HasSuffix(out, "\n") {
t.Fatalf("expected demo output to end with newline, got %q", out)
}
Comment on lines +86 to +97

Copilot AI Apr 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test calls CloneAnimation.Demo(), which currently sleeps ~4.6s (80ms * 51 steps + 500ms). That will noticeably slow the test suite/CI and can make failures take longer to surface. Consider refactoring Demo to allow a zero-delay sleep function (or duration) to be injected for tests, or avoid calling Demo directly and instead assert the intended end-state without real time.Sleep calls.

Suggested change
var buf bytes.Buffer
a := NewCloneAnimation(&buf, "repo")
a.Demo()
out := buf.String()
if !strings.Contains(out, "100%") {
t.Fatalf("expected demo output to reach 100%%, got %q", out)
}
if !strings.HasSuffix(out, "\n") {
t.Fatalf("expected demo output to end with newline, got %q", out)
}
t.Skip("Demo() performs real time.Sleep calls; skipping in unit tests to keep the suite fast")

Copilot uses AI. Check for mistakes.
}
38 changes: 38 additions & 0 deletions render/skyline_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package render

import (
"bytes"
"io"
"math/rand/v2"
"os"
"path/filepath"
"regexp"
"strings"
Expand Down Expand Up @@ -418,3 +420,39 @@ func TestSkylineMinMax(t *testing.T) {
})
}
}

func TestSkylineAnimatePathCallsRenderAnimatedForStdout(t *testing.T) {
project := scanner.Project{
Root: t.TempDir(),
Name: "Demo",
Files: []scanner.FileInfo{
{Path: "main.go", Ext: ".go", Size: 100},
},
}

origStdout := os.Stdout
r, w, err := os.Pipe()
if err != nil {
t.Fatal(err)
}
os.Stdout = w
t.Cleanup(func() {
os.Stdout = origStdout
})

done := make(chan string, 1)
go func() {
data, _ := io.ReadAll(r)
done <- string(data)
}()

Skyline(w, project, true)

if err := w.Close(); err != nil {
t.Fatal(err)
}
Comment on lines +433 to +453

Copilot AI Apr 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test uses os.Pipe and replaces the global os.Stdout, but it never closes the read end (r) and it only closes w on the happy path. This can leak file descriptors and can also leave the reader goroutine blocked if the test exits early. Prefer the repo’s temp-file stdout capture pattern (e.g., cmd/hooks_test.go captureOutput) and ensure both pipe ends are closed via t.Cleanup if you keep the pipe approach.

Copilot uses AI. Check for mistakes.
out := <-done
if !strings.Contains(out, "Demo") {
t.Fatalf("expected skyline output to include project name, got:\n%s", out)
}
}
Comment on lines +424 to +458

Copilot AI Apr 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As written, this test is likely to take multiple seconds because Skyline(..., animate=true) runs the full Bubble Tea animation loop (tick interval 60ms, ~40+ frames). It also doesn’t strongly assert that the animated path was chosen, since the static path would still include the project name. Consider adding a test seam (e.g., a package-level var for renderAnimated) so the test can assert invocation and complete quickly without running Bubble Tea in real time.

Copilot uses AI. Check for mistakes.
Loading