Skip to content

Commit 7113a16

Browse files
committed
test: address Copilot review comments on PR #172
- query: assert m.Match result instead of discarding it - fix: assert atomicWriteFile returns error; check temp-file prefix not length - ruledocs: use deterministic errFS type so ReadDir error tests are reliable - config: assert yaml.Unmarshal returns expected "bool or mapping" error - crossfilereferenceintegrity: rename misleading TestParseTarget_OpaqueURL test - tablereadability: remove dead _ = math.MaxFloat64 line - headingstyle: convert fmt.Printf exploratory tests to real assertions https://claude.ai/code/session_01DvP5H17ofGpHmR7DhSU438
1 parent 63102e4 commit 7113a16

7 files changed

Lines changed: 35 additions & 61 deletions

File tree

internal/config/config_test.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1577,10 +1577,8 @@ func TestUnmarshalYAML_NonScalarNonMappingValue(t *testing.T) {
15771577
input := "rules:\n line-length:\n - item1\n - item2\n"
15781578
var cfg Config
15791579
err := yaml.Unmarshal([]byte(input), &cfg)
1580-
// The error may occur during YAML unmarshalling.
1581-
// If the custom UnmarshalYAML triggers the "not bool or mapping" path, we get an error.
1582-
// Just check we don't panic.
1583-
_ = err
1580+
require.Error(t, err)
1581+
assert.Contains(t, err.Error(), "rule config must be a bool or a mapping")
15841582
}
15851583

15861584
// TestTopLevelKeySet_DocumentNodeEmpty exercises the

internal/fix/fix_coverage_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"os"
66
"path/filepath"
77
"runtime"
8+
"strings"
89
"testing"
910

1011
"github.com/jeduden/mdsmith/internal/config"
@@ -437,13 +438,14 @@ func TestAtomicWriteFile_TmpFileCleanedUpOnRenameFailure(t *testing.T) {
437438
targetDir := filepath.Join(dir, "target")
438439
require.NoError(t, os.Mkdir(targetDir, 0o755))
439440

440-
_ = atomicWriteFile(targetDir, []byte("data"), 0o644)
441+
err := atomicWriteFile(targetDir, []byte("data"), 0o644)
442+
require.Error(t, err)
441443

442444
// Verify no orphaned temp files remain in the directory after the failure.
443445
entries, err := os.ReadDir(dir)
444446
require.NoError(t, err)
445447
for _, e := range entries {
446-
assert.False(t, len(e.Name()) > len(".mdsmith-fix-"),
448+
assert.False(t, strings.HasPrefix(e.Name(), ".mdsmith-fix-"),
447449
"unexpected leftover temp file: %s", e.Name())
448450
}
449451
}

internal/query/query_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,10 +120,11 @@ func TestCollectPaths_NonStructCUE(t *testing.T) {
120120
// collectPaths should return nil and Match should use unification only.
121121
m, err := Compile(`>=1 & <=10`)
122122
require.NoError(t, err)
123-
// A non-struct schema: no paths to verify. Unification with a plain
124-
// JSON number value would succeed, but the front matter is a map.
125-
// The exact result depends on CUE unification semantics.
126-
_ = m.Match(map[string]any{"value": 5})
123+
// A non-struct schema has no paths to verify. Match therefore falls back
124+
// to unification only, and a map-shaped front matter value cannot unify
125+
// with a top-level numeric constraint.
126+
result := m.Match(map[string]any{"value": 5})
127+
assert.False(t, result, "non-struct CUE schema should not match a map value")
127128
}
128129

129130
// TestMatch_JSONMarshalError exercises the json.Marshal err != nil branch in Match.

internal/rules/crossfilereferenceintegrity/rule_morecoverage_test.go

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -197,16 +197,7 @@ func TestCheck_InvalidExcludeGlobReturnsConfigDiag(t *testing.T) {
197197

198198
// --- parseTarget: path == "" && u.Opaque != "" ---
199199
// An opaque URI like "mailto:user@example.com" has Opaque set.
200-
// But url.Parse of "mailto:user@example.com" sets Scheme="mailto", so it
201-
// returns false before reaching the opaque branch.
202-
// To reach opaque: need Scheme=="" and Opaque!="". This is an unusual URL
203-
// like "C:path" on Windows (opaque path). We construct it directly.
204-
func TestParseTarget_OpaqueURL(t *testing.T) {
205-
// url.Parse("C:relative") → {Opaque: "relative", Scheme: "C"} — has Scheme, returns false.
206-
// It's very hard to get Scheme="" and Opaque!="" via url.Parse in practice.
207-
// The only case is if url.Parse fails to identify a scheme but sets Opaque.
208-
// Actually this branch may be dead code; skip it and test something else.
209-
// Instead test a path with just a fragment but no hash prefix.
200+
func TestParseTarget_PlainRelativePath(t *testing.T) {
210201
target, ok := parseTarget("guide.md")
211202
require.True(t, ok)
212203
require.Equal(t, "guide.md", target.Path)
Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,53 +1,51 @@
11
package headingstyle
22

33
import (
4-
"fmt"
54
"testing"
65

76
"github.com/jeduden/mdsmith/internal/lint"
87
"github.com/yuin/goldmark/ast"
98
"github.com/yuin/goldmark/text"
109
)
1110

12-
func TestExploreHeadingWithManualChildren(t *testing.T) {
13-
// Try to craft a heading with Lines().Len() == 0 but with text children
14-
// by creating one manually and calling headingLine
15-
11+
func TestHeadingLine_ManualATXWithTextChild(t *testing.T) {
12+
// A manually constructed ATX heading with Lines().Len()==0 and a Text child;
13+
// headingLine should fall back to the child segment offset (line 1).
1614
src := []byte("# Title\n")
1715
f, err := lint.NewFile("test.md", src)
1816
if err != nil {
1917
t.Fatal(err)
2018
}
21-
_ = f
2219

23-
// Create a heading with no lines but manually attach a text child
2420
h := ast.NewHeading(1)
2521
textNode := ast.NewText()
2622
textNode.Segment = text.NewSegment(2, 7) // "Title" in "# Title\n"
2723
h.AppendChild(h, textNode)
2824

29-
fmt.Printf("Manual heading: Lines=%d, FirstChild=%T\n", h.Lines().Len(), h.FirstChild())
3025
line := headingLine(h, f)
31-
fmt.Printf("headingLine returned: %d\n", line)
26+
if line < 1 {
27+
t.Errorf("expected headingLine >= 1, got %d", line)
28+
}
3229
}
3330

34-
func TestExploreHeadingWithNonTextChild(t *testing.T) {
35-
// Heading with Lines=0 and first child is Emphasis (not Text), wrapping Text
31+
func TestHeadingLine_ManualATXWithEmphasisChild(t *testing.T) {
32+
// Heading with Lines==0 and first child is Emphasis wrapping Text;
33+
// headingLine should still return a valid line number (>= 1).
3634
src := []byte("# **bold**\n")
3735
f, err := lint.NewFile("test.md", src)
3836
if err != nil {
3937
t.Fatal(err)
4038
}
4139

42-
// Create heading manually
4340
h := ast.NewHeading(1)
4441
em := ast.NewEmphasis(2)
4542
textNode := ast.NewText()
46-
textNode.Segment = text.NewSegment(3, 7) // "bold" in "# **bold**\n" -- rough
43+
textNode.Segment = text.NewSegment(3, 7)
4744
em.AppendChild(em, textNode)
4845
h.AppendChild(h, em)
4946

50-
fmt.Printf("Manual heading with emphasis: Lines=%d\n", h.Lines().Len())
5147
line := headingLine(h, f)
52-
fmt.Printf("headingLine returned: %d (expected 1 for offset 3)\n", line)
48+
if line < 1 {
49+
t.Errorf("expected headingLine >= 1, got %d", line)
50+
}
5351
}

internal/rules/ruledocs_test.go

Lines changed: 10 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package rules
22

33
import (
4+
"fmt"
45
"io/fs"
56
"testing"
67
"testing/fstest"
@@ -9,6 +10,11 @@ import (
910
"github.com/stretchr/testify/require"
1011
)
1112

13+
// errFS is an fs.FS that always returns an error on Open, forcing ReadDir to fail.
14+
type errFS struct{}
15+
16+
func (errFS) Open(string) (fs.File, error) { return nil, fmt.Errorf("forced readdir error") }
17+
1218
func TestListRules_SortedByID(t *testing.T) {
1319
rules, err := ListRules()
1420
require.NoError(t, err, "ListRules: %v", err)
@@ -150,20 +156,8 @@ func TestListRulesFromFS_SkipsMissingStatus(t *testing.T) {
150156

151157
// listRulesFromFS: ReadDir error
152158
func TestListRulesFromFS_ReadDirError(t *testing.T) {
153-
// An empty fstest.MapFS with a sub-path causes ReadDir(".")
154-
// to succeed, but we can use a sub-FS that errors on ReadDir.
155-
// The simplest way: use an fs.Sub on a non-existent subdirectory.
156-
fsys := fstest.MapFS{
157-
"readme.txt": &fstest.MapFile{Data: []byte("not a dir")},
158-
}
159-
// fs.Sub on a file path causes ReadDir to fail.
160-
sub, err := fs.Sub(fsys, "readme.txt")
161-
if err == nil {
162-
_, err = listRulesFromFS(sub)
163-
// Whether it errors or not depends on the FS implementation;
164-
// the key is we exercise the ReadDir error path.
165-
_ = err
166-
}
159+
_, err := listRulesFromFS(errFS{})
160+
require.Error(t, err)
167161
}
168162

169163
// listRulesFromFS: non-directory entry → continue
@@ -202,17 +196,8 @@ func TestListRulesFromFS_SkipsEmptyDir(t *testing.T) {
202196

203197
// lookupRuleFromFS: listRulesFromFS error propagation
204198
func TestLookupRuleFromFS_PropagatesReadDirError(t *testing.T) {
205-
// Use an FS that returns an error on ReadDir.
206-
fsys := fstest.MapFS{
207-
"readme.txt": &fstest.MapFile{Data: []byte("file")},
208-
}
209-
sub, err := fs.Sub(fsys, "readme.txt")
210-
if err == nil {
211-
_, err = lookupRuleFromFS(sub, "anything")
212-
// Error may or may not occur depending on implementation;
213-
// this exercises the lookupRuleFromFS error propagation path.
214-
_ = err
215-
}
199+
_, err := lookupRuleFromFS(errFS{}, "anything")
200+
require.Error(t, err)
216201
}
217202

218203
// parseFrontMatter: missing ID → error

internal/rules/tablereadability/rule_coverage_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,6 @@ func TestColumnWidthRatio_OneEmptyOneNonEmpty(t *testing.T) {
322322
}
323323
ratio := tbl.columnWidthRatio()
324324
assert.True(t, math.IsInf(ratio, 1), "expected +Inf when min column avg is 0")
325-
_ = math.MaxFloat64 // suppress unused import warning
326325
}
327326

328327
// columnWidthRatio: only separator rows → columns==0 → return 0

0 commit comments

Comments
 (0)