Refuse a symbolic link under experiments/ (#61)

internal/check/check_test.go

@@ -22,12 +22,7 @@ import (
 func TestCases(t *testing.T) {
 	for name, want := range loadCases(t) {
 		t.Run(name, func(t *testing.T) {
-			root := filepath.Join(casesDir, name, "tree")
-			if _, err := os.Stat(root); err != nil {
-				t.Fatalf("case %s has no tree: %v", name, err)
-			}
-
-			got, err := Walk(root, fixedNow)
+			got, err := walkCase(t, name)
 			if err != nil {
 				t.Fatalf("walk failed: %v", err)
 			}
@@ -195,7 +190,7 @@ func TestFixtureBytesSurviveTheCheckout(t *testing.T) {
 // the source, and the reader is usually somebody who has just arrived.
 func TestARefusalNamesItsSubject(t *testing.T) {
 	for name := range loadCases(t) {
-		result, err := Walk(filepath.Join(casesDir, name, "tree"), fixedNow)
+		result, err := walkCase(t, name)
 		if err != nil {
 			t.Fatalf("walk of %s failed: %v", name, err)
 		}
@@ -231,7 +226,7 @@ func TestWalkWritesNothing(t *testing.T) {
 	before := fingerprint(t, casesDir)
 
 	for name := range loadCases(t) {
-		if _, err := Walk(filepath.Join(casesDir, name, "tree"), fixedNow); err != nil {
+		if _, err := walkCase(t, name); err != nil {
 			t.Fatalf("walk of %s failed: %v", name, err)
 		}
 	}

internal/check/decision.go

@@ -1,9 +1,9 @@
 package check
 
 import (
+	"errors"
 	"fmt"
-	"os"
-	"path/filepath"
+	"io/fs"
 	"regexp"
 	"sort"
 	"strings"
@@ -80,14 +80,13 @@ var supersession = regexp.MustCompile(`(?i)supersedes\s+(?:record\s+)?(\d{4})`)
 // along with what it refused. A tree with no decisions directory reads none,
 // which is an ordinary state for a fixture tree and is reported rather than
 // treated as an error.
-func refuseDecisions(root string) (int, []Refusal, error) {
-	dir := filepath.Join(root, filepath.FromSlash(DecisionsDir))
-	entries, err := os.ReadDir(dir)
+func refuseDecisions(fsys fs.FS, root string) (int, []Refusal, error) {
+	entries, err := fs.ReadDir(fsys, DecisionsDir)
 	if err != nil {
-		if os.IsNotExist(err) {
+		if errors.Is(err, fs.ErrNotExist) {
 			return 0, nil, nil
 		}
-		return 0, nil, fmt.Errorf("cannot read %s: %w", dir, err)
+		return 0, nil, fmt.Errorf("cannot read %s: %w", at(root, DecisionsDir), err)
 	}
 
 	var refusals []Refusal
@@ -96,9 +95,9 @@ func refuseDecisions(root string) (int, []Refusal, error) {
 	present := make(map[string]bool)
 	var records []string
 
-	// os.ReadDir sorts by filename, so the record that reports a shared number
-	// is the later one of the pair every time this runs rather than whichever
-	// the filesystem happened to hand over first.
+	// A directory listing arrives sorted by filename, so the record that
+	// reports a shared number is the later one of the pair every time this
+	// runs rather than whichever the filesystem happened to hand over first.
 	for _, entry := range entries {
 		if entry.IsDir() {
 			continue
@@ -113,7 +112,8 @@ func refuseDecisions(root string) (int, []Refusal, error) {
 	}
 
 	for _, name := range records {
-		path := filepath.Join(dir, name)
+		inside := DecisionsDir + "/" + name
+		path := at(root, inside)
 		number := decisionFileName.FindStringSubmatch(name)[1]
 
 		if first, taken := numbers[number]; taken {
@@ -126,7 +126,7 @@ func refuseDecisions(root string) (int, []Refusal, error) {
 			numbers[number] = name
 		}
 
-		data, err := os.ReadFile(path)
+		data, err := fs.ReadFile(fsys, inside)
 		if err != nil {
 			return read, nil, fmt.Errorf("cannot read %s: %w", path, err)
 		}

internal/check/hardware.go

@@ -1,9 +1,9 @@
 package check
 
 import (
+	"errors"
 	"fmt"
 	"io/fs"
-	"os"
 	"path/filepath"
 	"strings"
 )
@@ -62,7 +62,7 @@ const (
 // A record whose bytes do not parse as a record is not judged here, for the
 // reason refuseState and refuseHeaderDates both give: nothing can read a field
 // out of a file that has no header.
-func refuseHardware(experiment, path string, data []byte) ([]Refusal, error) {
+func refuseHardware(fsys fs.FS, inside, experiment, path string, data []byte) ([]Refusal, error) {
 	record, err := ParseRecord(data)
 	if err != nil {
 		return nil, nil
@@ -82,7 +82,7 @@ func refuseHardware(experiment, path string, data []byte) ([]Refusal, error) {
 		}}, nil
 	}
 
-	registered, err := harnessTestsUnder(experiment)
+	registered, err := harnessTestsUnder(fsys, inside, experiment)
 	if err != nil {
 		return nil, err
 	}
@@ -127,37 +127,33 @@ func refuseHardware(experiment, path string, data []byte) ([]Refusal, error) {
 // directory nobody wrote by hand. A directory below the bound is not descended
 // into, and TheTreeIsDeeperThanTheWalkReads is what refuses the tree that
 // reaches it.
-func harnessTestsUnder(experiment string) ([]string, error) {
+func harnessTestsUnder(fsys fs.FS, inside, experiment string) ([]string, error) {
 	var registered []string
 
-	// filepath.WalkDir does not follow symbolic links, so a link pointing out
-	// of the experiment is reported as a link and never descended into.
-	err := filepath.WalkDir(experiment, func(path string, entry fs.DirEntry, err error) error {
+	// A tree walk does not follow symbolic links, so a link pointing out of
+	// the experiment is reported as a link and never descended into. What
+	// refuses such a link is the stray-record walk, which reaches every path
+	// under experiments/; this reading leaves it alone and registers nothing
+	// for it, because a name is all this reads and a link's name says nothing
+	// about what it points at.
+	err := fs.WalkDir(fsys, inside, func(name string, entry fs.DirEntry, err error) error {
 		if err != nil {
 			return err
 		}
 		if entry.IsDir() {
-			deeper, err := depthOf(experiment, path)
-			if err != nil {
-				return err
-			}
-			if deeper > WalkDepthBound {
+			if depthOf(name)-depthOf(inside) > WalkDepthBound {
 				return fs.SkipDir
 			}
 			return nil
 		}
 		if !entry.Type().IsRegular() || !strings.HasSuffix(entry.Name(), HarnessTestSuffix) {
 			return nil
 		}
-		relative, err := filepath.Rel(experiment, path)
-		if err != nil {
-			return fmt.Errorf("cannot place %s inside %s: %w", path, experiment, err)
-		}
-		registered = append(registered, filepath.ToSlash(relative))
+		registered = append(registered, strings.TrimPrefix(name, inside+"/"))
 		return nil
 	})
 	if err != nil {
-		if os.IsNotExist(err) {
+		if errors.Is(err, fs.ErrNotExist) {
 			return nil, nil
 		}
 		return nil, fmt.Errorf("cannot walk %s: %w", experiment, err)

internal/check/hardware_test.go

@@ -59,7 +59,8 @@ func TestWhatCountsAsRegisteredWithTheHarness(t *testing.T) {
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			registered, err := harnessTestsUnder(filepath.Join(casesDir, tc.name, "tree", "experiments", "one"))
+			tree := filepath.Join(casesDir, tc.name, "tree")
+			registered, err := harnessTestsUnder(os.DirFS(tree), "experiments/one", filepath.Join(tree, "experiments", "one"))
 			if err != nil {
 				t.Fatalf("reading the directory failed: %v", err)
 			}
@@ -75,7 +76,8 @@ func TestWhatCountsAsRegisteredWithTheHarness(t *testing.T) {
 // experiments/ with no record at all is refused by ExperimentHasNoRecord, and a
 // second refusal about its harness files would name a repair nobody needs.
 func TestADirectoryThatIsNotThereRegistersNothing(t *testing.T) {
-	registered, err := harnessTestsUnder(filepath.Join(casesDir, "no-experiments-directory", "tree", "experiments", "nothing-here"))
+	tree := filepath.Join(casesDir, "no-experiments-directory", "tree")
+	registered, err := harnessTestsUnder(os.DirFS(tree), "experiments/nothing-here", filepath.Join(tree, "experiments", "nothing-here"))
 	if err != nil {
 		t.Fatalf("reading a directory that is not there failed: %v", err)
 	}

internal/check/harness_test.go

@@ -19,10 +19,39 @@
 //	testdata/cases/<name>/near-neighbour      the case that differs by the
 //	                                          smallest legal change, required
 //	                                          of a case that refuses
+//	testdata/cases/<name>/links               one path per line, each an entry
+//	                                          the walk is shown as a symbolic
+//	                                          link, absent from most cases
 //
 // This file decides that layout. Nothing restates it, so there is nothing to
 // drift against it.
 //
+// THE LAST ONE IS A DEPARTURE FROM THE PARAGRAPH ABOVE AND IT IS DELIBERATE.
+// Every other case is bytes in the repository and nothing else. A link is not,
+// because a checkout cannot be relied on to carry one: creating a symbolic
+// link on Windows needs SeCreateSymbolicLinkPrivilege, which an ordinary
+// account does not hold, and os.Symlink there returns windows error 1314. The
+// measurement is on issue #61. A link stored as a tracked entry arrives on such
+// a checkout as an ordinary file holding the target as text, so a case built
+// that way would ask the runner about a link on one platform and about a text
+// file on another while declaring one answer for both. Record 0012 runs the
+// suite on windows/amd64 and #57 keeps every platform running the same suite
+// with nothing skipped, so neither the tracked link nor a link built at run
+// time is available.
+//
+// What a declared link costs and what it buys. The bytes under tree/ are still
+// exactly what the walk read, and the only thing the harness supplies is the
+// type of one directory entry. What that leaves unproved is that a link made
+// by an operating system arrives as an entry of that type, which is the
+// standard library's behaviour rather than this runner's. What it proves is
+// the whole of what this runner decides: what it does when it meets one.
+//
+// The pair to read together is a-symbolic-link-under-experiments and its near
+// neighbour an-ordinary-file-where-a-link-would-be. Their trees are identical
+// byte for byte, and the only difference between them is the line declaring
+// the entry a link, so the refusal is shown to be about the link and not about
+// the name, the place or the bytes.
+//
 // THE BOUND ON WHAT ANY OF THIS PROVES. Every comparison here is over which
 // properties were refused, and never over which line inside the runner refused
 // them. Two refusal sites producing one property are indistinguishable to
@@ -38,6 +67,7 @@ package check
 
 import (
 	"fmt"
+	"io/fs"
 	"os"
 	"path/filepath"
 	"sort"
@@ -165,6 +195,141 @@ func readExpectation(t *testing.T, dir string) expectation {
 	return exp
 }
 
+// walkCase walks one case's tree and returns what the runner found. Every test
+// that runs a case goes through it, so the decision about how a case is
+// presented to the walk is made once.
+func walkCase(t *testing.T, name string) (Result, error) {
+	t.Helper()
+
+	tree := filepath.Join(casesDir, name, "tree")
+	if _, err := os.Stat(tree); err != nil {
+		t.Fatalf("case %s has no tree: %v", name, err)
+	}
+
+	var fsys fs.FS = os.DirFS(tree)
+	if links := linksDeclaredBy(t, name); len(links) > 0 {
+		fsys = treeWithLinks{FS: fsys, links: links}
+	}
+	return walk(fsys, tree, fixedNow)
+}
+
+// linksDeclaredBy reads the entries a case declares as symbolic links. A case
+// declaring none is the ordinary case and reads as an empty set rather than as
+// a missing file it has to apologise for.
+func linksDeclaredBy(t *testing.T, name string) map[string]bool {
+	t.Helper()
+
+	data, err := os.ReadFile(filepath.Join(casesDir, name, "links"))
+	if os.IsNotExist(err) {
+		return nil
+	}
+	if err != nil {
+		t.Fatalf("case %s: %v", name, err)
+	}
+
+	links := make(map[string]bool)
+	for _, line := range strings.Split(string(data), "\n") {
+		if line = strings.TrimSpace(line); line != "" {
+			links[line] = true
+		}
+	}
+	return links
+}
+
+// treeWithLinks is a case's tree with some of its entries reported as symbolic
+// links. Everything else is read from the tree on disk, unchanged, so the case
+// is still the files a reader can open.
+type treeWithLinks struct {
+	fs.FS
+	links map[string]bool
+}
+
+// ReadDir is the one thing this overrides, because a directory listing is
+// where the walk learns what an entry is.
+func (t treeWithLinks) ReadDir(name string) ([]fs.DirEntry, error) {
+	entries, err := fs.ReadDir(t.FS, name)
+	if err != nil {
+		return nil, err
+	}
+	for i, entry := range entries {
+		inside := entry.Name()
+		if name != "." {
+			inside = name + "/" + inside
+		}
+		if t.links[inside] {
+			entries[i] = declaredLink{DirEntry: entry}
+		}
+	}
+	return entries, nil
+}
+
+// declaredLink is one directory entry the case declares to be a symbolic link.
+// The name and the underlying file are the tree's own; the type is what this
+// supplies.
+type declaredLink struct {
+	fs.DirEntry
+}
+
+func (d declaredLink) Type() fs.FileMode { return fs.ModeSymlink }
+
+func (d declaredLink) IsDir() bool { return false }
+
+func (d declaredLink) Info() (fs.FileInfo, error) {
+	info, err := d.DirEntry.Info()
+	if err != nil {
+		return nil, err
+	}
+	return linkInfo{FileInfo: info}, nil
+}
+
+// linkInfo carries the same answers as the file on disk apart from the one bit
+// that says what it is.
+type linkInfo struct {
+	fs.FileInfo
+}
+
+func (l linkInfo) Mode() fs.FileMode { return l.FileInfo.Mode()&^fs.ModeType | fs.ModeSymlink }
+
+func (l linkInfo) IsDir() bool { return false }
+
+// TestADeclaredLinkIsReportedAsOne holds the harness's own half of the link
+// case. The refusal it feeds is proved by the case; that the harness really
+// puts a link in front of the walk is proved here, because a declaration this
+// dropped in silence would leave the case green against an ordinary file and
+// the refusal unproved while the suite said otherwise.
+func TestADeclaredLinkIsReportedAsOne(t *testing.T) {
+	const name = "a-symbolic-link-under-experiments"
+
+	links := linksDeclaredBy(t, name)
+	if len(links) == 0 {
+		t.Fatalf("case %s declares no link, so the case for the link refusal is not the case it claims to be", name)
+	}
+
+	tree := filepath.Join(casesDir, name, "tree")
+	plain, err := fs.ReadDir(os.DirFS(tree), ExperimentsDir)
+	if err != nil {
+		t.Fatalf("cannot read %s: %v", tree, err)
+	}
+	declared, err := fs.ReadDir(treeWithLinks{FS: os.DirFS(tree), links: links}, ExperimentsDir)
+	if err != nil {
+		t.Fatalf("cannot read %s: %v", tree, err)
+	}
+
+	for i := range plain {
+		inside := ExperimentsDir + "/" + plain[i].Name()
+		wantLink := links[inside]
+		if got := declared[i].Type()&fs.ModeSymlink != 0; got != wantLink {
+			t.Errorf("%s is reported as a link %v, want %v", inside, got, wantLink)
+		}
+		if wantLink && plain[i].Type()&fs.ModeSymlink != 0 {
+			t.Errorf("%s is already a link on disk, so this case proves nothing about a declaration", inside)
+		}
+		if declared[i].Name() != plain[i].Name() {
+			t.Errorf("the declaration renamed %s to %s", plain[i].Name(), declared[i].Name())
+		}
+	}
+}
+
 func mustAtoi(t *testing.T, dir, value string) int {
 	t.Helper()
 	n, err := strconv.Atoi(value)

testdata/cases/a-symbolic-link-under-experiments/expected

@@ -0,0 +1,4 @@
+directories 1
+records 1
+experiments present
+decisions absent

testdata/cases/a-symbolic-link-under-experiments/expected-refusals

@@ -0,0 +1 @@
+experiments-holds-a-symbolic-link

testdata/cases/a-symbolic-link-under-experiments/links

@@ -0,0 +1 @@
+experiments/up

testdata/cases/a-symbolic-link-under-experiments/near-neighbour

@@ -0,0 +1 @@
+an-ordinary-file-where-a-link-would-be

testdata/cases/a-symbolic-link-under-experiments/tree/experiments/one/EXPERIMENT.md

@@ -0,0 +1,3 @@
+# One
+
+The smallest tree the walk can count as an experiment with a record.