From 6bb013e6d05df9af7b87b0d4f1a4ffbdc423ac15 Mon Sep 17 00:00:00 2001 From: Pieczasz Date: Wed, 16 Sep 2026 08:50:50 +0000 Subject: [PATCH 1/5] test: raise statement coverage above 90% Add CLI scan/fail-on/cardinality/version tests, Materialize coverage, pattern/resolve edge cases, and a thin cmd/tarsier run() for testability. --- cli/check_cardinality_test.go | 50 ++++++++++++ cli/failon_test.go | 61 ++++++++++++-- cli/scan_test.go | 141 ++++++++++++++++++++++++++++++++- cli/version.go | 23 ++++-- cli/version_test.go | 33 ++++++++ cmd/tarsier/main.go | 14 +++- cmd/tarsier/main_test.go | 25 ++++++ engine/pattern/runner_test.go | 136 ++++++++++++++++++++++++++++++- engine/resolve/resolve_test.go | 46 +++++++++++ rules/pack_test.go | 52 ++++++++++++ 10 files changed, 562 insertions(+), 19 deletions(-) create mode 100644 cmd/tarsier/main_test.go create mode 100644 rules/pack_test.go diff --git a/cli/check_cardinality_test.go b/cli/check_cardinality_test.go index 52be5a0..1cc2dc9 100644 --- a/cli/check_cardinality_test.go +++ b/cli/check_cardinality_test.go @@ -40,3 +40,53 @@ func TestIsCardinalityRule(t *testing.T) { t.Fatal("unexpected match") } } + +func TestCheckCardinalityCleanTree(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "README.md"), []byte("ok\n"), 0o600); err != nil { + t.Fatal(err) + } + root := NewRootCommand() + buf := &bytes.Buffer{} + root.SetOut(buf) + root.SetArgs([]string{"check-cardinality", dir}) + if err := root.ExecuteContext(context.Background()); err != nil { + t.Fatal(err) + } + if !strings.Contains(buf.String(), "ok: no cardinality findings") { + t.Fatalf("want ok banner, got %q", buf.String()) + } +} + +func TestCheckCardinalityFailOn(t *testing.T) { + if _, err := os.Stat(filepath.Join("..", "fixtures", "_badshop")); err != nil { + t.Skip("fixtures not available") + } + root := NewRootCommand() + root.SetOut(&bytes.Buffer{}) + root.SetErr(&bytes.Buffer{}) + root.SetArgs([]string{ + "check-cardinality", + "--fail-on", "warning", + filepath.Join("..", "fixtures", "_badshop"), + }) + err := root.ExecuteContext(context.Background()) + if err == nil { + t.Fatal("want fail-on threshold error") + } + if !strings.Contains(err.Error(), "findings at or above") { + t.Fatalf("got %v", err) + } +} + +func TestCheckCardinalityInvalidFailOn(t *testing.T) { + dir := t.TempDir() + root := NewRootCommand() + root.SetOut(&bytes.Buffer{}) + root.SetErr(&bytes.Buffer{}) + root.SetArgs([]string{"check-cardinality", "--fail-on", "loud", dir}) + err := root.ExecuteContext(context.Background()) + if err == nil || !strings.Contains(err.Error(), "invalid --fail-on") { + t.Fatalf("want invalid --fail-on, got %v", err) + } +} diff --git a/cli/failon_test.go b/cli/failon_test.go index bd38557..50a9b0a 100644 --- a/cli/failon_test.go +++ b/cli/failon_test.go @@ -10,12 +10,63 @@ import ( func TestFailOnRank(t *testing.T) { t.Parallel() - if _, err := failOnRank("loud"); err == nil || !strings.Contains(err.Error(), "invalid --fail-on") { - t.Fatalf("typo'd threshold must fail, got %v", err) + tests := []struct { + level string + want int + err bool + }{ + {level: "", want: 0}, + {level: failOnNone, want: 0}, + {level: "hint", want: 1}, + {level: failOnInfo, want: 2}, + {level: failOnWarning, want: 3}, + {level: failOnError, want: 4}, + {level: "loud", err: true}, } - n, err := failOnRank(failOnNone) - if err != nil || n != 0 { - t.Fatalf("none = %d, %v; want 0, nil", n, err) + for _, tt := range tests { + t.Run(tt.level, func(t *testing.T) { + t.Parallel() + n, err := failOnRank(tt.level) + if tt.err { + if err == nil || !strings.Contains(err.Error(), "invalid --fail-on") { + t.Fatalf("want invalid --fail-on error, got %v", err) + } + return + } + if err != nil || n != tt.want { + t.Fatalf("failOnRank(%q) = %d, %v; want %d, nil", tt.level, n, err, tt.want) + } + }) + } +} + +func TestSeverityRank(t *testing.T) { + t.Parallel() + + tests := []struct { + sev string + want int + }{ + {sev: "hint", want: 1}, + {sev: "info", want: 2}, + {sev: "warning", want: 3}, + {sev: "error", want: 4}, + {sev: "unknown", want: 0}, + {sev: "", want: 0}, + } + for _, tt := range tests { + if got := severityRank(tt.sev); got != tt.want { + t.Errorf("severityRank(%q) = %d, want %d", tt.sev, got, tt.want) + } + } +} + +func TestFailOnThresholdErrorMessage(t *testing.T) { + t.Parallel() + + err := failOnThresholdError{threshold: "warning", count: 3} + if got := err.Error(); got != "3 findings at or above warning" { + t.Fatalf("Error() = %q", got) } } diff --git a/cli/scan_test.go b/cli/scan_test.go index e0e7d41..08468df 100644 --- a/cli/scan_test.go +++ b/cli/scan_test.go @@ -2,6 +2,7 @@ package cli import ( "bytes" + "errors" "os" "path/filepath" "strings" @@ -122,7 +123,14 @@ func TestSourcePath(t *testing.T) { t.Parallel() dir := t.TempDir() - file := filepath.Join(dir, "sub", "a.go") + sub := filepath.Join(dir, "sub") + if err := os.MkdirAll(sub, 0o750); err != nil { + t.Fatal(err) + } + file := filepath.Join(sub, "a.go") + if err := os.WriteFile(file, []byte("package p\n"), 0o600); err != nil { + t.Fatal(err) + } if got := sourcePath(dir, filepath.Join("sub", "a.go")); got != file { t.Errorf("dir root joins: got %q want %q", got, file) @@ -130,11 +138,142 @@ func TestSourcePath(t *testing.T) { if got := sourcePath(file, file); got != file { t.Errorf("single-file root returns the input: got %q want %q", got, file) } + if got := sourcePath(file, "a.go"); got != file { + t.Errorf("single-file root + basename: got %q want %q", got, file) + } + if got := sourcePath(file, "other.go"); got != filepath.Join(sub, "other.go") { + t.Errorf("single-file root + sibling: got %q", got) + } if got := sourcePath("/repo", "/abs/a.go"); got != "/abs/a.go" { t.Errorf("absolute finding passes through: got %q", got) } } +func TestRenderRejectsUnknownFormat(t *testing.T) { + t.Parallel() + + var out bytes.Buffer + err := render(&out, nil, "yaml", ".") + if err == nil || !strings.Contains(err.Error(), "invalid --output") { + t.Fatalf("want invalid --output error, got %v", err) + } +} + +func TestScanCommand(t *testing.T) { + badshop := filepath.Join("..", "fixtures", "_badshop") + if _, err := os.Stat(badshop); err != nil { + t.Skip("fixtures not available") + } + godeep := filepath.Join("..", "fixtures", "godeep") + clean := t.TempDir() + if err := os.WriteFile(filepath.Join(clean, "README.md"), []byte("ok\n"), 0o600); err != nil { + t.Fatal(err) + } + + t.Run("pattern scan on fixture finds issues", func(t *testing.T) { + root := NewRootCommand() + buf := &bytes.Buffer{} + root.SetOut(buf) + root.SetArgs([]string{"scan", "--engine", "pattern", "--output", "json", badshop}) + if err := root.Execute(); err != nil { + t.Fatal(err) + } + if !strings.Contains(buf.String(), `"rule"`) { + t.Fatalf("expected findings JSON, got %q", buf.String()) + } + }) + + t.Run("go engine on godeep", func(t *testing.T) { + root := NewRootCommand() + buf := &bytes.Buffer{} + root.SetOut(buf) + root.SetArgs([]string{"scan", "--engine", "go", "--output", "text", godeep}) + if err := root.Execute(); err != nil { + t.Fatal(err) + } + if !strings.Contains(buf.String(), "findings") { + t.Fatalf("expected text summary, got %q", buf.String()) + } + }) + + t.Run("clean tree with fail-on stays quiet", func(t *testing.T) { + root := NewRootCommand() + buf := &bytes.Buffer{} + root.SetOut(buf) + root.SetArgs([]string{"scan", "--engine", "pattern", "--fail-on", "warning", clean}) + if err := root.Execute(); err != nil { + t.Fatal(err) + } + if !strings.Contains(buf.String(), "0 findings") { + t.Fatalf("got %q", buf.String()) + } + }) + + t.Run("invalid engine", func(t *testing.T) { + root := NewRootCommand() + root.SetOut(&bytes.Buffer{}) + root.SetErr(&bytes.Buffer{}) + root.SetArgs([]string{"scan", "--engine", "wasm", clean}) + err := root.Execute() + if err == nil || !strings.Contains(err.Error(), "invalid --engine") { + t.Fatalf("want invalid --engine, got %v", err) + } + }) + + t.Run("invalid fail-on", func(t *testing.T) { + root := NewRootCommand() + root.SetOut(&bytes.Buffer{}) + root.SetErr(&bytes.Buffer{}) + root.SetArgs([]string{"scan", "--fail-on", "loud", clean}) + err := root.Execute() + if err == nil || !strings.Contains(err.Error(), "invalid --fail-on") { + t.Fatalf("want invalid --fail-on, got %v", err) + } + }) + + t.Run("fail-on trips on findings", func(t *testing.T) { + root := NewRootCommand() + root.SetOut(&bytes.Buffer{}) + root.SetErr(&bytes.Buffer{}) + root.SetArgs([]string{"scan", "--engine", "pattern", "--fail-on", "warning", badshop}) + err := root.Execute() + if err == nil { + t.Fatal("want fail-on threshold error") + } + var thresh failOnThresholdError + if !errors.As(err, &thresh) && !strings.Contains(err.Error(), "findings at or above") { + t.Fatalf("want threshold error, got %v", err) + } + }) + + t.Run("explicit rules config", func(t *testing.T) { + cfg, err := filepath.Abs(filepath.Join("..", "sgconfig.yml")) + if err != nil { + t.Fatal(err) + } + root := NewRootCommand() + buf := &bytes.Buffer{} + root.SetOut(buf) + root.SetArgs([]string{"scan", "--rules", cfg, "--engine", "pattern", clean}) + if err := root.Execute(); err != nil { + t.Fatal(err) + } + }) + + t.Run("html output", func(t *testing.T) { + root := NewRootCommand() + buf := &bytes.Buffer{} + root.SetOut(buf) + root.SetArgs([]string{"scan", "--engine", "pattern", "--output", "html", clean}) + if err := root.Execute(); err != nil { + t.Fatal(err) + } + if !strings.Contains(buf.String(), "") { + t.Fatalf("want html, got %q", buf.String()) + } + }) +} + func writeSource(t *testing.T, dir, body string) { t.Helper() if err := os.WriteFile(filepath.Join(dir, "a.go"), []byte(body), 0o600); err != nil { diff --git a/cli/version.go b/cli/version.go index fd4a3d9..c3153a5 100644 --- a/cli/version.go +++ b/cli/version.go @@ -3,6 +3,7 @@ package cli import ( "encoding/json" "fmt" + "io" "runtime/debug" "github.com/spf13/cobra" @@ -29,21 +30,27 @@ func newVersionCommand() *cobra.Command { enc.SetIndent("", " ") return enc.Encode(info) } - dirty := "" - if info.Dirty { - dirty = "-dirty" - } - _, err := fmt.Fprintf(cmd.OutOrStdout(), "%s %s%s (%s)\n", - info.Module, info.Revision, dirty, info.Go) + _, err := io.WriteString(cmd.OutOrStdout(), formatVersionText(info)) return err }, } } +func formatVersionText(info buildInfo) string { + dirty := "" + if info.Dirty { + dirty = "-dirty" + } + return fmt.Sprintf("%s %s%s (%s)\n", info.Module, info.Revision, dirty, info.Go) +} + func readBuildInfo() buildInfo { + return buildInfoFrom(debug.ReadBuildInfo()) +} + +func buildInfoFrom(raw *debug.BuildInfo, ok bool) buildInfo { info := buildInfo{Module: "tarsier", Revision: "unknown", Go: "unknown"} - raw, ok := debug.ReadBuildInfo() - if !ok { + if !ok || raw == nil { return info } info.Module, info.Go = raw.Main.Path, raw.GoVersion diff --git a/cli/version_test.go b/cli/version_test.go index 57b36e8..8031616 100644 --- a/cli/version_test.go +++ b/cli/version_test.go @@ -2,6 +2,7 @@ package cli import ( "bytes" + "runtime/debug" "strings" "testing" ) @@ -34,3 +35,35 @@ func TestVersionRespectsOutputFormat(t *testing.T) { }) } } + +func TestBuildInfoFrom(t *testing.T) { + t.Parallel() + + if got := buildInfoFrom(nil, false); got.Revision != "unknown" || got.Module != "tarsier" { + t.Fatalf("missing build info: %+v", got) + } + + raw := &debug.BuildInfo{ + GoVersion: "go1.22.0", + Main: debug.Module{Path: "github.com/Pieczasz/tarsier"}, + Settings: []debug.BuildSetting{ + {Key: "vcs.revision", Value: "abc123"}, + {Key: "vcs.time", Value: "2026-01-02T03:04:05Z"}, + {Key: "vcs.modified", Value: "true"}, + {Key: "other", Value: "ignored"}, + }, + } + got := buildInfoFrom(raw, true) + if got.Module != "github.com/Pieczasz/tarsier" || got.Revision != "abc123" || + got.Time != "2026-01-02T03:04:05Z" || !got.Dirty || got.Go != "go1.22.0" { + t.Fatalf("unexpected build info: %+v", got) + } + text := formatVersionText(got) + if !strings.Contains(text, "abc123-dirty") { + t.Fatalf("dirty text = %q", text) + } + clean := formatVersionText(buildInfo{Module: "tarsier", Revision: "deadbeef", Go: "go1.22"}) + if strings.Contains(clean, "-dirty") || !strings.Contains(clean, "deadbeef") { + t.Fatalf("clean text = %q", clean) + } +} diff --git a/cmd/tarsier/main.go b/cmd/tarsier/main.go index 750bc8b..e73e0b2 100644 --- a/cmd/tarsier/main.go +++ b/cmd/tarsier/main.go @@ -3,14 +3,22 @@ package main import ( "fmt" + "io" "os" "github.com/Pieczasz/tarsier/cli" ) func main() { - if err := cli.NewRootCommand().Execute(); err != nil { - fmt.Fprintln(os.Stderr, "tarsier:", err) - os.Exit(1) + os.Exit(run(os.Args[1:], os.Stderr)) +} + +func run(args []string, errOut io.Writer) int { + root := cli.NewRootCommand() + root.SetArgs(args) + if err := root.Execute(); err != nil { + fmt.Fprintln(errOut, "tarsier:", err) + return 1 } + return 0 } diff --git a/cmd/tarsier/main_test.go b/cmd/tarsier/main_test.go new file mode 100644 index 0000000..439428d --- /dev/null +++ b/cmd/tarsier/main_test.go @@ -0,0 +1,25 @@ +package main + +import ( + "bytes" + "strings" + "testing" +) + +func TestRunVersion(t *testing.T) { + var errBuf bytes.Buffer + if code := run([]string{"version"}, &errBuf); code != 0 { + t.Fatalf("exit %d, stderr %q", code, errBuf.String()) + } +} + +func TestRunReportsErrors(t *testing.T) { + var errBuf bytes.Buffer + code := run([]string{"--log-level", "loud", "version"}, &errBuf) + if code != 1 { + t.Fatalf("exit %d, want 1", code) + } + if !strings.Contains(errBuf.String(), "tarsier:") { + t.Fatalf("stderr = %q", errBuf.String()) + } +} diff --git a/engine/pattern/runner_test.go b/engine/pattern/runner_test.go index 3857014..27809f8 100644 --- a/engine/pattern/runner_test.go +++ b/engine/pattern/runner_test.go @@ -2,6 +2,7 @@ package pattern import ( "context" + "errors" "fmt" "os" "os/exec" @@ -492,8 +493,28 @@ cat %q >&2 exit %d `, version, outFile, errFile, exitCode) - bin := filepath.Join(dir, "ast-grep") - if err := os.WriteFile(bin, []byte(script), 0o700); err != nil { + return writeExecScript(t, dir, "ast-grep", script) +} + +func fakeAstGrepVersionOnly(t *testing.T, versionLine string) string { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("fake binary is a POSIX shell script") + } + dir := t.TempDir() + return writeExecScript(t, dir, "ast-grep", fmt.Sprintf("#!/bin/sh\necho %q\n", versionLine)) +} + +// writeExecScript writes a shell script via rename so exec never races a +// still-open writer (ETXTBSY on overlayfs). +func writeExecScript(t *testing.T, dir, name, script string) string { + t.Helper() + tmp := filepath.Join(dir, name+".tmp") + bin := filepath.Join(dir, name) + if err := os.WriteFile(tmp, []byte(script), 0o700); err != nil { + t.Fatal(err) + } + if err := os.Rename(tmp, bin); err != nil { t.Fatal(err) } return bin @@ -519,3 +540,114 @@ func TestFingerprintsSurviveAnInsertionAbove(t *testing.T) { t.Error("fingerprint changed after an unrelated match was added above") } } + +func TestTruncateBase(t *testing.T) { + t.Parallel() + + if got := truncateBase("short"); got != "short" { + t.Fatalf("got %q", got) + } + multi := "line one\nline two\nline three" + if got := truncateBase(multi); strings.Contains(got, "\n") { + t.Fatalf("want flattened, got %q", got) + } + long := strings.Repeat("字", maxSymbolLen+10) + got := truncateBase(long) + if n := len([]rune(got)); n != maxSymbolLen { + t.Fatalf("len=%d want %d (%q)", n, maxSymbolLen, got) + } + if !strings.HasSuffix(got, "…") { + t.Fatalf("want ellipsis, got %q", got) + } +} + +func TestRelPathEdges(t *testing.T) { + t.Parallel() + + if got := relPath("api/metrics.go", "metrics.go"); got != "api/metrics.go" { + t.Fatalf("basename under file root: %q", got) + } + if got := relPath("/repo", "/repo"); got != "/repo" { + t.Fatalf("same path: %q", got) + } + if got := relPath("/repo", "/repo/a.go"); got != "a.go" { + t.Fatalf("child: %q", got) + } +} + +func TestWaitErrBranches(t *testing.T) { + t.Parallel() + + var stderr strings.Builder + if err := waitErr(context.Background(), nil, &stderr, false); err != nil { + t.Fatal(err) + } + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if err := waitErr(ctx, context.Canceled, &stderr, false); err == nil || !strings.Contains(err.Error(), "canceled") { + t.Fatalf("want canceled, got %v", err) + } + + dctx, dcancel := context.WithTimeout(context.Background(), 0) + <-dctx.Done() + dcancel() + if err := waitErr(dctx, context.DeadlineExceeded, &stderr, false); err == nil || !strings.Contains(err.Error(), "timed out") { + t.Fatalf("want timeout, got %v", err) + } + + if err := waitErr(context.Background(), errors.New("boom"), &stderr, false); err == nil || !strings.Contains(err.Error(), "ast-grep failed") { + t.Fatalf("want failed without stderr detail, got %v", err) + } +} + +func TestCheckVersionUnparseable(t *testing.T) { + t.Parallel() + + bin := fakeAstGrepVersionOnly(t, "not-a-version") + err := checkVersion(context.Background(), bin) + if err == nil || !strings.Contains(err.Error(), "cannot parse") { + t.Fatalf("got %v", err) + } +} + +func TestScanSingleFileTarget(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + file := filepath.Join(dir, "m.go") + if err := os.WriteFile(file, []byte("package p\n"), 0o600); err != nil { + t.Fatal(err) + } + r := &Runner{Bin: fakeAstGrep(t, "", "", minVersion, 0)} + got, err := r.Scan(context.Background(), file) + if err != nil { + t.Fatal(err) + } + if len(got) != 0 { + t.Fatalf("got %v", got) + } +} + +func TestScanMergesCardinalityExtras(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + src := []byte(`package shop; +class M { + private static final String[] LABELS = { "user_id" }; + void f(){ Counter.build().labelNames(LABELS).register(); } +} +`) + if err := os.WriteFile(filepath.Join(dir, "M.java"), src, 0o600); err != nil { + t.Fatal(err) + } + r := &Runner{Bin: fakeAstGrep(t, "", "", minVersion, 0)} + got, err := r.Scan(context.Background(), dir) + if err != nil { + t.Fatal(err) + } + if len(got) == 0 { + t.Fatal("want cardinality extras from Java const labels") + } +} diff --git a/engine/resolve/resolve_test.go b/engine/resolve/resolve_test.go index 7cf4b54..0f7b91a 100644 --- a/engine/resolve/resolve_test.go +++ b/engine/resolve/resolve_test.go @@ -1,6 +1,7 @@ package resolve import ( + "errors" "os" "path/filepath" "strings" @@ -599,3 +600,48 @@ class Leak { func write(dir, name string, body []byte) error { return os.WriteFile(filepath.Join(dir, name), body, 0o600) } + +func TestPathInsideRootEdges(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + abs, err := filepath.Abs(dir) + if err != nil { + t.Fatal(err) + } + if !pathInsideRoot(abs, filepath.Join(dir, "a.go")) { + // file need not exist for Abs; EvalSymlinks may fail — create it + if err := os.WriteFile(filepath.Join(dir, "a.go"), []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + if !pathInsideRoot(abs, filepath.Join(dir, "a.go")) { + t.Fatal("expected inside") + } + } + if pathInsideRoot(abs, filepath.Join(dir, "missing-dangling-xyz")) { + t.Fatal("dangling path must be rejected") + } + outside := t.TempDir() + if err := os.WriteFile(filepath.Join(outside, "b.go"), []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + if pathInsideRoot(abs, filepath.Join(outside, "b.go")) { + t.Fatal("outside path must be rejected") + } +} + +func TestExtrasForFileNonJavaPython(t *testing.T) { + t.Parallel() + got, err := extrasForFile("x.go", "x.go", []byte("package p\n")) + if err != nil || len(got) != 0 { + t.Fatalf("%v %v", got, err) + } +} + +func TestVisitCardinalityPathWalkError(t *testing.T) { + t.Parallel() + _, skip, err := visitCardinalityPath("/tmp", "/tmp", "/tmp/x", nil, errors.New("walk boom")) + if !skip || err == nil { + t.Fatalf("want skip+err, got skip=%v err=%v", skip, err) + } +} diff --git a/rules/pack_test.go b/rules/pack_test.go new file mode 100644 index 0000000..67ca6dd --- /dev/null +++ b/rules/pack_test.go @@ -0,0 +1,52 @@ +package rules + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestMaterializeWritesPackAndConfig(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + config, err := Materialize(dir) + if err != nil { + t.Fatal(err) + } + if filepath.Base(config) != "sgconfig.yml" { + t.Fatalf("config path = %q", config) + } + body, err := os.ReadFile(config) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(body), "ruleDirs:") || !strings.Contains(string(body), "utilDirs:") { + t.Fatalf("sgconfig content unexpected: %s", body) + } + rulesDir := filepath.Join(dir, "rules") + entries, err := os.ReadDir(rulesDir) + if err != nil { + t.Fatal(err) + } + if len(entries) == 0 { + t.Fatal("expected embedded rule directories under rules/") + } + utilsDir := filepath.Join(dir, "utils") + if _, err := os.Stat(filepath.Join(utilsDir, "unbounded-label.yml")); err != nil { + t.Fatalf("embedded util missing: %v", err) + } +} + +func TestMaterializeRejectsNonDirectory(t *testing.T) { + t.Parallel() + + path := filepath.Join(t.TempDir(), "not-a-dir") + if err := os.WriteFile(path, []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + if _, err := Materialize(path); err == nil { + t.Fatal("Materialize into a file path must fail") + } +} From d074d63e51091bc2fc8a5af2a06121c3fb963c07 Mon Sep 17 00:00:00 2001 From: Pieczasz Date: Wed, 16 Sep 2026 10:26:22 +0000 Subject: [PATCH 2/5] test: cover Materialize error paths and cmd/tarsier main Raise rules and cmd/tarsier packages to 100% statement coverage. --- cmd/tarsier/main.go | 5 ++++- cmd/tarsier/main_test.go | 35 +++++++++++++++++++++++++++++++++++ rules/pack_test.go | 24 ++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 1 deletion(-) diff --git a/cmd/tarsier/main.go b/cmd/tarsier/main.go index e73e0b2..a3fd122 100644 --- a/cmd/tarsier/main.go +++ b/cmd/tarsier/main.go @@ -9,8 +9,11 @@ import ( "github.com/Pieczasz/tarsier/cli" ) +// exitFunc is swapped in tests so main can be exercised without killing the process. +var exitFunc = os.Exit + func main() { - os.Exit(run(os.Args[1:], os.Stderr)) + exitFunc(run(os.Args[1:], os.Stderr)) } func run(args []string, errOut io.Writer) int { diff --git a/cmd/tarsier/main_test.go b/cmd/tarsier/main_test.go index 439428d..928b51b 100644 --- a/cmd/tarsier/main_test.go +++ b/cmd/tarsier/main_test.go @@ -2,6 +2,7 @@ package main import ( "bytes" + "os" "strings" "testing" ) @@ -23,3 +24,37 @@ func TestRunReportsErrors(t *testing.T) { t.Fatalf("stderr = %q", errBuf.String()) } } + +func TestRunHelp(t *testing.T) { + var errBuf bytes.Buffer + if code := run([]string{"--help"}, &errBuf); code != 0 { + t.Fatalf("exit %d, stderr %q", code, errBuf.String()) + } +} + +func TestRunUnknownCommand(t *testing.T) { + var errBuf bytes.Buffer + code := run([]string{"not-a-real-command"}, &errBuf) + if code != 1 { + t.Fatalf("exit %d, want 1; stderr %q", code, errBuf.String()) + } + if !strings.Contains(errBuf.String(), "tarsier:") { + t.Fatalf("stderr = %q", errBuf.String()) + } +} + +func TestMainInvokesRun(t *testing.T) { + oldArgs := os.Args + oldExit := exitFunc + defer func() { + os.Args = oldArgs + exitFunc = oldExit + }() + var code int + exitFunc = func(c int) { code = c } + os.Args = []string{"tarsier", "version"} + main() + if code != 0 { + t.Fatalf("main exit %d", code) + } +} diff --git a/rules/pack_test.go b/rules/pack_test.go index 67ca6dd..aac9c7a 100644 --- a/rules/pack_test.go +++ b/rules/pack_test.go @@ -50,3 +50,27 @@ func TestMaterializeRejectsNonDirectory(t *testing.T) { t.Fatal("Materialize into a file path must fail") } } + +func TestMaterializeRejectsUtilsPathConflict(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "utils"), []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + if _, err := Materialize(dir); err == nil { + t.Fatal("Materialize must fail when utils is a file") + } +} + +func TestMaterializeRejectsConfigPathConflict(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + if err := os.Mkdir(filepath.Join(dir, "sgconfig.yml"), 0o700); err != nil { + t.Fatal(err) + } + if _, err := Materialize(dir); err == nil { + t.Fatal("Materialize must fail when sgconfig.yml is a directory") + } +} From 2f01afb9c93cb985f80cddd2a96a537075989199 Mon Sep 17 00:00:00 2001 From: Pieczasz Date: Fri, 18 Sep 2026 18:42:14 +0000 Subject: [PATCH 3/5] chore: scrub comment punctuation and clarify a few whys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace em/en dashes and fancy arrows with ASCII. Extract packagesContainGo so the empty-Go success path is named instead of narrated. Trim finding status comments to the non-obvious bits. Co-authored-by: Bartłomiej Piekarz --- analyzers/otelresource/otelresource.go | 4 ++-- cli/check_cardinality_test.go | 2 +- cli/failon.go | 2 +- engine/golang/runner.go | 20 +++++++++++--------- engine/golang/runner_test.go | 2 +- engine/resolve/resolve.go | 8 ++++---- engine/resolve/resolve_test.go | 2 +- finding/finding.go | 8 +++----- rules/logs/missing-trace-correlation.go.yml | 4 ++-- 9 files changed, 26 insertions(+), 26 deletions(-) diff --git a/analyzers/otelresource/otelresource.go b/analyzers/otelresource/otelresource.go index f704454..71766a5 100644 --- a/analyzers/otelresource/otelresource.go +++ b/analyzers/otelresource/otelresource.go @@ -57,7 +57,7 @@ func run(pass *analysis.Pass) (any, error) { //nolint:gocyclo // wrapper + direc return false }) - // Pass 2: call sites of those wrappers (one hop) — medium confidence. + // Pass 2: call sites of those wrappers (one hop) - medium confidence. insp.Nodes([]ast.Node{(*ast.CallExpr)(nil)}, func(n ast.Node, push bool) bool { if !push { return false @@ -70,7 +70,7 @@ func run(pass *analysis.Pass) (any, error) { //nolint:gocyclo // wrapper + direc if fn == nil || !wrappers[fn] { return false } - // Don't double-report the resource.New site inside the wrapper body. + // Skip the inner resource.New; pass 1 already reported it. if isResourceNew(pass, call) { return false } diff --git a/cli/check_cardinality_test.go b/cli/check_cardinality_test.go index 1cc2dc9..b37b4e0 100644 --- a/cli/check_cardinality_test.go +++ b/cli/check_cardinality_test.go @@ -22,7 +22,7 @@ func TestCheckCardinalityFiltersRules(t *testing.T) { } out := buf.String() if !strings.Contains(out, "metrics/high-cardinality-label") && !strings.Contains(out, "high-cardinality") { - // text format prints message, not always rule id — accept either + // text format prints message, not always rule id - accept either if !strings.Contains(out, "unbounded") && !strings.Contains(out, "billable") { t.Fatalf("expected cardinality output, got %q", out) } diff --git a/cli/failon.go b/cli/failon.go index 5999362..9fdeecf 100644 --- a/cli/failon.go +++ b/cli/failon.go @@ -14,7 +14,7 @@ const ( failOnError = "error" ) -// failOnError is returned after a successful scan when --fail-on is tripped. +// failOnThresholdError is returned after a successful scan when --fail-on is tripped. type failOnThresholdError struct { threshold string count int diff --git a/engine/golang/runner.go b/engine/golang/runner.go index 266a7d9..6197147 100644 --- a/engine/golang/runner.go +++ b/engine/golang/runner.go @@ -94,15 +94,8 @@ func (r *Runner) Scan(root string) ([]finding.Finding, error) { //nolint:gocyclo if len(pkgs) == 0 { return nil, nil } - // No Go files at all (e.g. JS-only tree) — not an error. - hasGo := false - for _, p := range pkgs { - if len(p.GoFiles) > 0 || len(p.CompiledGoFiles) > 0 { - hasGo = true - break - } - } - if !hasGo { + if !packagesContainGo(pkgs) { + // Mixed-language trees still Load; empty Go set is success, not failure. return nil, nil } @@ -131,6 +124,15 @@ func (r *Runner) Scan(root string) ([]finding.Finding, error) { //nolint:gocyclo return out, nil } +func packagesContainGo(pkgs []*packages.Package) bool { + for _, p := range pkgs { + if len(p.GoFiles) > 0 || len(p.CompiledGoFiles) > 0 { + return true + } + } + return false +} + // runAnalyzer runs one analyzer. Some third-party analyzers panic on partial // dependency graphs (ctrlflow); isolate so one bad wrapper cannot kill the scan. func runAnalyzer(a *analysis.Analyzer, pkgs []*packages.Package, root string) (out []finding.Finding) { diff --git a/engine/golang/runner_test.go b/engine/golang/runner_test.go index 99cb421..7d96fdf 100644 --- a/engine/golang/runner_test.go +++ b/engine/golang/runner_test.go @@ -93,7 +93,7 @@ func TestDefaultsValidate(t *testing.T) { if err := analysis.Validate(Defaults()); err != nil { t.Fatal(err) } - // Full default set on the tiny fixture — exercises wrapper registration. + // Full default set on the tiny fixture - exercises wrapper registration. got, err := (&Runner{}).Scan(godeepDir(t)) if err != nil { t.Fatal(err) diff --git a/engine/resolve/resolve.go b/engine/resolve/resolve.go index 306cafe..5265f4a 100644 --- a/engine/resolve/resolve.go +++ b/engine/resolve/resolve.go @@ -1,5 +1,5 @@ // Package resolve is the thin per-file import/symbol layer for the pattern -// tier. It is deliberately not a type checker: alias→package maps plus +// tier. It is deliberately not a type checker: alias->package maps plus // file-level string / string-slice literals (and Python Exception params). package resolve @@ -23,9 +23,9 @@ type Lit struct { type File struct { Path string Lang string - Imports map[string]string // local name → import path - Strings map[string][]Lit // const/var name → string literals (incl. slice elems) - Params map[string]string // Python: param → type annotation text + Imports map[string]string // local name -> import path + Strings map[string][]Lit // const/var name -> string literals (incl. slice elems) + Params map[string]string // Python: param -> type annotation text } const ( diff --git a/engine/resolve/resolve_test.go b/engine/resolve/resolve_test.go index 0f7b91a..6d3476a 100644 --- a/engine/resolve/resolve_test.go +++ b/engine/resolve/resolve_test.go @@ -610,7 +610,7 @@ func TestPathInsideRootEdges(t *testing.T) { t.Fatal(err) } if !pathInsideRoot(abs, filepath.Join(dir, "a.go")) { - // file need not exist for Abs; EvalSymlinks may fail — create it + // file need not exist for Abs; EvalSymlinks may fail - create it if err := os.WriteFile(filepath.Join(dir, "a.go"), []byte("x"), 0o600); err != nil { t.Fatal(err) } diff --git a/finding/finding.go b/finding/finding.go index 9f6526f..9dafad2 100644 --- a/finding/finding.go +++ b/finding/finding.go @@ -11,14 +11,12 @@ import ( // Finding lifecycle states. Baselines drop known findings from output; // suppressions keep them, marked. const ( - // StatusOpen is a finding reported as-is. StatusOpen = "open" - // StatusSuppressed is a finding quieted by an inline tarsier:ignore - // comment. It stays in JSON output so consumers can audit suppressions. + // StatusSuppressed stays in JSON so consumers can audit quieted findings. StatusSuppressed = "suppressed" ) -// Finding is a single normalized observability gap emitted by an analyzer. +// Finding is the shared schema across analyzers, reports, store, and PR checks. type Finding struct { Fingerprint string `json:"fingerprint"` Rule string `json:"rule"` @@ -62,7 +60,7 @@ type WorkflowRef struct { Criticality string `json:"criticality"` } -// Fingerprint hashes the finding identity for baselines and suppression. +// Fingerprint is stable across line drift; baselines and suppressions key on it. func Fingerprint(rule, module, relPath, symbol string) string { sum := sha256.Sum256([]byte(strings.Join([]string{ rule, module, strings.ReplaceAll(relPath, "\\", "/"), symbol, diff --git a/rules/logs/missing-trace-correlation.go.yml b/rules/logs/missing-trace-correlation.go.yml index ab62221..e1c39e8 100644 --- a/rules/logs/missing-trace-correlation.go.yml +++ b/rules/logs/missing-trace-correlation.go.yml @@ -9,11 +9,11 @@ metadata: confidence: medium spec_ref: instrumentation-score/LOG-001 -message: "Structured logger is used alongside OTel without a log↔trace bridge." +message: "Structured logger is used alongside OTel without a log<->trace bridge." note: | Without trace_id/span_id on log records, logs cannot join traces. When this module already imports go.opentelemetry.io/otel and log/slog (or zap), wire - go.opentelemetry.io/contrib/bridges/otelslog (or otelzap) — do not call the + go.opentelemetry.io/contrib/bridges/otelslog (or otelzap) - do not call the Logs Bridge API from application code. Precision ceiling: a bridge registered in another package is invisible. From 7755b39355990936c24b8d23735f704f5e074cf1 Mon Sep 17 00:00:00 2001 From: Pieczasz Date: Fri, 18 Sep 2026 19:09:17 +0000 Subject: [PATCH 4/5] fix: drop unused gocyclo nolint on Runner.Scan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit golangci-lint nolintlint failed CI on PR #2 after packagesContainGo extraction lowered cyclomatic complexity. Co-authored-by: Bartłomiej Piekarz --- engine/golang/runner.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/golang/runner.go b/engine/golang/runner.go index 6197147..26c67eb 100644 --- a/engine/golang/runner.go +++ b/engine/golang/runner.go @@ -70,7 +70,7 @@ func Defaults() []*analysis.Analyzer { // Scan type-checks packages under root and returns normalized findings. // Root must be a Go module (or contain one). Non-Go trees return nil, nil. -func (r *Runner) Scan(root string) ([]finding.Finding, error) { //nolint:gocyclo // load + filter + per-analyzer loop +func (r *Runner) Scan(root string) ([]finding.Finding, error) { analyzers := r.Analyzers if analyzers == nil { From eb4c23bad3799cd948c4f33b6ba78b5fa5b0b57b Mon Sep 17 00:00:00 2001 From: Pieczasz Date: Fri, 18 Sep 2026 20:00:48 +0000 Subject: [PATCH 5/5] fix: gofumpt Runner.Scan after nolint removal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop the blank line left behind when the unused gocyclo directive was removed. Co-authored-by: Bartłomiej Piekarz --- engine/golang/runner.go | 1 - 1 file changed, 1 deletion(-) diff --git a/engine/golang/runner.go b/engine/golang/runner.go index 26c67eb..dd3f290 100644 --- a/engine/golang/runner.go +++ b/engine/golang/runner.go @@ -71,7 +71,6 @@ func Defaults() []*analysis.Analyzer { // Scan type-checks packages under root and returns normalized findings. // Root must be a Go module (or contain one). Non-Go trees return nil, nil. func (r *Runner) Scan(root string) ([]finding.Finding, error) { - analyzers := r.Analyzers if analyzers == nil { analyzers = Defaults()