From bc754eaecbda87b819b162083dc71fb5e9732653 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Tue, 14 Jul 2026 11:24:02 +0200 Subject: [PATCH] =?UTF-8?q?test:=20coverage=20wave=202=20=E2=80=94=20utili?= =?UTF-8?q?ty=20packages=20toward=2095%?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First wave-2 batch (post-merge of the coverage sprint), driving the small self-contained packages up with real, measured tests: - pathutil: 91.7% -> 100% (ExpandHome's no-home + unknown-~user fallbacks) - slug: 94.3% -> 97.1% (Derive fallback + collision-truncation branches) - schema: 84.5% -> 94.4% (unwrap/errors_as helpers; FormatErrors same-Path tiebreak; flatten nil-guard) - config: 75.8% -> 93.4% (Current; Profile nil-map; Dir/Path/Load/Save/clearAll error branches via cleared HOME; Load 2nd-unmarshal; migrateV1 error; Save rename-onto-dir) pathutil + slug clear 95%. config + schema plateau just under, blocked ONLY by by-construction-uncoverable branches: config's atomic-write defensive I/O (Chmod/Write/Close on a fresh temp file) + the unreachable MarshalIndent check; schema's NewV1Validator "embedded schema malformed" defense-in-depth (can't happen — CI drift check + go:embed guarantee it) + ValidateYAML's "jsonschema/v6 always returns a *ValidationError" defensive else. Reaching those means faking the filesystem / removing idiomatic error handling — deliberately not done. make ci green. Co-Authored-By: Claude Opus 4.8 --- internal/config/config_coverage_test.go | 157 ++++++++++++++++++ internal/pathutil/expand_coverage_test.go | 18 ++ internal/schema/errhelpers_coverage_test.go | 28 ++++ internal/schema/formaterrors_coverage_test.go | 28 ++++ internal/slug/derive_coverage_test.go | 39 +++++ 5 files changed, 270 insertions(+) create mode 100644 internal/config/config_coverage_test.go create mode 100644 internal/pathutil/expand_coverage_test.go create mode 100644 internal/schema/errhelpers_coverage_test.go create mode 100644 internal/schema/formaterrors_coverage_test.go create mode 100644 internal/slug/derive_coverage_test.go diff --git a/internal/config/config_coverage_test.go b/internal/config/config_coverage_test.go new file mode 100644 index 00000000..4cd908cb --- /dev/null +++ b/internal/config/config_coverage_test.go @@ -0,0 +1,157 @@ +package config + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// clearHomeAndConfigDir removes every source of a config dir so Dir()/Path() +// fail — the lever that covers the error-propagation branches of Dir, Path, +// Load, Save and clearAll (os.UserHomeDir errors when $HOME is empty). +func clearHomeAndConfigDir(t *testing.T) { + t.Helper() + t.Setenv("TRACEBLOC_CONFIG_DIR", "") + t.Setenv("HOME", "") +} + +func TestCurrent_EmptyAndSet(t *testing.T) { + // No current env → a fresh empty profile (the read-only "not signed in" view), + // never nil. + if got := (&Config{}).Current(); got == nil || *got != (Profile{}) { + t.Errorf("Current() with no env = %+v, want a fresh empty Profile", got) + } + // With a current env → that env's live profile. + c := &Config{CurrentEnv: "dev", Profiles: map[string]*Profile{"dev": {Token: "t"}}} + if got := c.Current(); got == nil || got.Token != "t" { + t.Errorf("Current() = %+v, want the dev profile", got) + } +} + +func TestProfile_NilMapAndReuse(t *testing.T) { + c := &Config{} // nil Profiles map + p := c.Profile("dev") + if p == nil { + t.Fatal("Profile must create and store an empty profile") + } + if c.Profiles == nil { + t.Error("Profile must initialize the Profiles map") + } + p.Token = "x" + if c.Profile("dev") != p { + t.Error("Profile must return the same live pointer on re-fetch") + } +} + +func TestDir_DefaultAndError(t *testing.T) { + // Default (no override) → /.tracebloc. + t.Setenv("TRACEBLOC_CONFIG_DIR", "") + home := t.TempDir() + t.Setenv("HOME", home) + dir, err := Dir() + if err != nil { + t.Fatalf("Dir with a home set: %v", err) + } + if want := filepath.Join(home, ".tracebloc"); dir != want { + t.Errorf("Dir() = %q, want %q", dir, want) + } + // No override AND no home → UserHomeDir fails. + clearHomeAndConfigDir(t) + if _, err := Dir(); err == nil { + t.Error("Dir() with no home must error") + } +} + +func TestPath_Error(t *testing.T) { + clearHomeAndConfigDir(t) + if _, err := Path(); err == nil { + t.Error("Path() must propagate Dir()'s error") + } +} + +// TestLoad_ConfigUnmarshalError covers the SECOND unmarshal (into Config): the +// probe parses (version 2, non-empty profiles → no migrate) but the full decode +// fails because profiles is a JSON array, not an object. +func TestLoad_ConfigUnmarshalError(t *testing.T) { + dir := t.TempDir() + t.Setenv("TRACEBLOC_CONFIG_DIR", dir) + if err := os.WriteFile(filepath.Join(dir, "config.json"), + []byte(`{"version":2,"profiles":[1,2]}`), 0o600); err != nil { + t.Fatal(err) + } + if _, err := Load(); err == nil || !strings.Contains(err.Error(), "parsing") { + t.Errorf("a v2 file with a non-object profiles must fail to parse, got %v", err) + } +} + +// TestLoad_NullProfilesGetsEmptyMap covers the `c.Profiles == nil` → empty-map +// arm (a v2 file that omits the profiles object). +func TestLoad_NullProfilesGetsEmptyMap(t *testing.T) { + dir := t.TempDir() + t.Setenv("TRACEBLOC_CONFIG_DIR", dir) + if err := os.WriteFile(filepath.Join(dir, "config.json"), + []byte(`{"version":2,"current_env":"dev"}`), 0o600); err != nil { + t.Fatal(err) + } + c, err := Load() + if err != nil { + t.Fatalf("Load: %v", err) + } + if c.Profiles == nil { + t.Error("Load must initialize a nil Profiles map to empty") + } +} + +func TestLoad_HomeError(t *testing.T) { + clearHomeAndConfigDir(t) + if _, err := Load(); err == nil { + t.Error("Load() must propagate Path()'s error") + } +} + +// TestMigrateV1_UnmarshalError covers migrateV1's own decode-failure arm: a file +// that probes as v1 (no version, no profiles) but whose fields don't fit the v1 +// struct (env as a number, not a string). +func TestMigrateV1_UnmarshalError(t *testing.T) { + dir := t.TempDir() + t.Setenv("TRACEBLOC_CONFIG_DIR", dir) + if err := os.WriteFile(filepath.Join(dir, "config.json"), + []byte(`{"env":123}`), 0o600); err != nil { + t.Fatal(err) + } + if _, err := Load(); err == nil || !strings.Contains(err.Error(), "parsing") { + t.Errorf("a v1 record with a non-string env must fail to migrate-parse, got %v", err) + } +} + +func TestSave_ErrorBranches(t *testing.T) { + t.Run("Dir error propagates", func(t *testing.T) { + clearHomeAndConfigDir(t) + if err := (&Config{CurrentEnv: "dev", Profiles: map[string]*Profile{"dev": {Token: "t"}}}).Save(); err == nil { + t.Error("Save() with no home must error") + } + }) + t.Run("rename onto a non-empty directory fails", func(t *testing.T) { + dir := t.TempDir() + t.Setenv("TRACEBLOC_CONFIG_DIR", dir) + // config.json as a NON-EMPTY directory → the final os.Rename(tmp, path) fails. + cfgDir := filepath.Join(dir, "config.json") + if err := os.Mkdir(cfgDir, 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(cfgDir, "child"), []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + if err := (&Config{CurrentEnv: "dev", Profiles: map[string]*Profile{"dev": {Token: "t"}}}).Save(); err == nil { + t.Error("Save must fail when the rename target is a non-empty directory") + } + }) +} + +func TestClearAll_HomeError(t *testing.T) { + clearHomeAndConfigDir(t) + if err := clearAll(); err == nil { + t.Error("clearAll() must propagate Path()'s error") + } +} diff --git a/internal/pathutil/expand_coverage_test.go b/internal/pathutil/expand_coverage_test.go new file mode 100644 index 00000000..f23c5e6b --- /dev/null +++ b/internal/pathutil/expand_coverage_test.go @@ -0,0 +1,18 @@ +package pathutil + +import "testing" + +// TestExpandHome_FallbackBranches covers ExpandHome's two "give up and return the +// path unchanged" arms: an unresolvable ~/ (no home) and an unknown ~user. +func TestExpandHome_FallbackBranches(t *testing.T) { + // "~/x" with no home → UserHomeDir errors → path returned unchanged. + t.Setenv("HOME", "") + if got := ExpandHome("~/x"); got != "~/x" { + t.Errorf("ExpandHome(~/x) with no home = %q, want it unchanged", got) + } + // "~/x" → user.Lookup fails → path returned unchanged. + const p = "~nosuchuser_zzz_qqq/data" + if got := ExpandHome(p); got != p { + t.Errorf("ExpandHome(%q) = %q, want it unchanged (unknown user)", p, got) + } +} diff --git a/internal/schema/errhelpers_coverage_test.go b/internal/schema/errhelpers_coverage_test.go new file mode 100644 index 00000000..f7a6a65d --- /dev/null +++ b/internal/schema/errhelpers_coverage_test.go @@ -0,0 +1,28 @@ +package schema + +import ( + "errors" + "fmt" + "testing" + + "github.com/santhosh-tekuri/jsonschema/v6" +) + +// TestErrorsAsAndUnwrap covers the two tiny error-chain helpers directly: unwrap +// (wrapping → inner, non-wrapping → nil) and errors_as walking a chain that +// holds no *ValidationError (→ false), exercising the full unwrap walk to nil. +func TestErrorsAsAndUnwrap(t *testing.T) { + inner := errors.New("in") + if got := unwrap(fmt.Errorf("w: %w", inner)); got != inner { + t.Errorf("unwrap(%%w-wrapped) = %v, want the inner error", got) + } + if got := unwrap(errors.New("plain")); got != nil { + t.Errorf("unwrap(non-wrapping) = %v, want nil", got) + } + + var target *jsonschema.ValidationError + chain := fmt.Errorf("a: %w", fmt.Errorf("b: %w", errors.New("c"))) + if errors_as(chain, &target) { + t.Error("errors_as must be false when the chain holds no *ValidationError") + } +} diff --git a/internal/schema/formaterrors_coverage_test.go b/internal/schema/formaterrors_coverage_test.go new file mode 100644 index 00000000..ee5d9a5a --- /dev/null +++ b/internal/schema/formaterrors_coverage_test.go @@ -0,0 +1,28 @@ +package schema + +import ( + "strings" + "testing" +) + +// TestFormatErrors_Tiebreak covers FormatErrors' secondary sort key: when two +// violations share a Path, ordering falls to Message (deterministic output). +func TestFormatErrors_Tiebreak(t *testing.T) { + out := FormatErrors([]ValidationError{ + {Path: "spec.x", Message: "zeta"}, + {Path: "spec.x", Message: "alpha"}, + }) + if !strings.Contains(out, "alpha") || !strings.Contains(out, "zeta") { + t.Fatalf("both messages should render:\n%s", out) + } + if strings.Index(out, "alpha") > strings.Index(out, "zeta") { + t.Errorf("same-Path violations must sort by Message (alpha before zeta):\n%s", out) + } +} + +// TestFlattenValidationError_Nil covers the empty-tree guard. +func TestFlattenValidationError_Nil(t *testing.T) { + if got := flattenValidationError(nil); got != nil { + t.Errorf("flattenValidationError(nil) = %v, want nil", got) + } +} diff --git a/internal/slug/derive_coverage_test.go b/internal/slug/derive_coverage_test.go new file mode 100644 index 00000000..9a245b21 --- /dev/null +++ b/internal/slug/derive_coverage_test.go @@ -0,0 +1,39 @@ +package slug + +import ( + "strings" + "testing" +) + +// TestDerive_FallbackAndTruncation covers Derive's fallback + collision-suffix +// branches (the paths TestDerive doesn't reach): empty-slug error, fallback use, +// both-empty → raw fallback, and the max-length truncation when a numbered +// suffix must still fit within MaxLabelLength. +func TestDerive_FallbackAndTruncation(t *testing.T) { + if _, err := Derive("", nil, ""); err == nil { + t.Error("empty slug + empty fallback must error") + } + if got, err := Derive("", nil, "fallback"); err != nil || got != "fallback" { + t.Errorf("Derive(empty, fallback) = %q, %v; want fallback", got, err) + } + // name AND fallback both slugify to empty → base becomes the raw fallback. + if got, err := Derive("!!!", nil, "@@@"); err != nil || got != "@@@" { + t.Errorf("Derive(both-empty-slug) = %q, %v; want the raw fallback @@@", got, err) + } + // Short-base collision → numbered suffix (end>len(base) clamp). + if got, err := Derive("Lab", []string{"lab"}, ""); err != nil || got != "lab-2" { + t.Errorf("Derive collision = %q, %v; want lab-2", got, err) + } + // Max-length-base collision → the base is truncated so "-2" fits within cap. + big := strings.Repeat("a", MaxLabelLength+10) + got, err := Derive(big, []string{Slugify(big)}, "") + if err != nil { + t.Fatalf("Derive(max-length collision): %v", err) + } + if len(got) > MaxLabelLength { + t.Errorf("Derive must keep the handle within MaxLabelLength (%d), got %d: %q", MaxLabelLength, len(got), got) + } + if !strings.HasSuffix(got, "-2") { + t.Errorf("Derive(collision) should carry a -2 suffix, got %q", got) + } +}