Strict mode for testexec structure

index.go

@@ -16,27 +16,28 @@ import (
 func (doc *Document) BuildDirIndex() {
 	doc.DirEnt = &DirEnt{}
 	for _, hunk := range doc.DataHunks {
-		doc.DirEnt.fill(strings.Split(hunk.Name, "/"), hunk.Hunk)
+		doc.DirEnt.fill(strings.Split(hunk.Name, "/"), 0, hunk.Hunk)
 	}
 }
 
-func (dirent *DirEnt) fill(pathSegs []string, hunk Hunk) {
-	if len(pathSegs) == 0 {
+func (dirent *DirEnt) fill(pathSegs []string, pathIdx int, hunk Hunk) {
+	if pathIdx >= len(pathSegs) {
 		dirent.Hunk = &hunk
 		return
 	}
 	if dirent.Children == nil {
 		dirent.Children = make(map[string]*DirEnt)
 	}
-	if next, exists := dirent.Children[pathSegs[0]]; exists {
-		next.fill(pathSegs[1:], hunk)
-	} else {
-		l := len(dirent.ChildrenList)
-		child := DirEnt{
-			Name: pathSegs[0],
-		}
-		dirent.ChildrenList = append(dirent.ChildrenList, &child)
-		dirent.Children[pathSegs[0]] = &child
-		dirent.ChildrenList[l].fill(pathSegs[1:], hunk)
+	if next, exists := dirent.Children[pathSegs[pathIdx]]; exists {
+		next.fill(pathSegs, pathIdx+1, hunk)
+		return
 	}
+
+	l := len(dirent.ChildrenList)
+	dirent.ChildrenList = append(dirent.ChildrenList, &DirEnt{
+		Name: pathSegs[pathIdx],
+		Path: strings.Join(pathSegs[:pathIdx+1], "/"),
+	})
+	dirent.Children[pathSegs[pathIdx]] = dirent.ChildrenList[l]
+	dirent.ChildrenList[l].fill(pathSegs, pathIdx+1, hunk)
 }

testexec/invalidexercise.md

@@ -0,0 +1,37 @@
+testexec invalid file
+===
+
+Shows examples of invalid testexec files
+
+---
+
+Blocks cannot test combined output and stdout
+
+[testmark]:# (stdout-combo/script)
+```
+  echo "hi"
+```
+[testmark]:# (stdout-combo/output)
+```
+hi
+```
+[testmark]:# (stdout-combo/stdout)
+```
+hi
+```
+
+---
+
+Similarly, blocks may not test combined output and stderr
+
+[testmark]:# (stderr-combo/script)
+```
+echo "hi"
+```
+[testmark]:# (stderr-combo/output)
+```
+hi
+```
+[testmark]:# (stderr-combo/stderr)
+```
+```

testexec/selfexercise.md

@@ -99,3 +99,4 @@ cat - | sed 's/ is/ was/'  | sed s/will/should/
 ```
 this was stdin and should be echoed
 ```
+

testexec/strictexercise.md

@@ -0,0 +1,47 @@
+testexec strict file
+===
+
+Shows examples of poorly formed testexec files. These will be skipped if strict mode is disabled.
+
+---
+
+A hunk with an empty intermediate step will not execute.
+
+[testmark]:# (parent/script)
+```
+  # no-op
+```
+
+[testmark]:# (parent/then-missing-script/then-another-thing/script)
+```
+  # no-op, this will not be reached because it missing executable blocks in parent steps
+```
+
+---
+
+Recursion won't happen if child blocks don't begin with `then-`
+Any unrecognized pattern will be an error in strict mode.
+
+[testmark]:# (norecurse/script)
+```
+  # no-op
+```
+[testmark]:# (norecurse/not-a-then-statement/script)
+```
+  # no-op, this script will not run because the subdirectory does not begin with "then-"
+```
+
+---
+
+Blocks can't contain both a `script` and a `sequence` child
+
+[testmark]:# (multiexec/script)
+```
+  # no-op, this script will not run because it has both types of exec blocks
+```
+
+[testmark]:# (multiexec/sequence)
+```
+  # no-op, this script will not run because it has both types of exec blocks
+```
+

testexec/testexec.go

@@ -68,6 +68,8 @@ type Tester struct {
 	AssertFn
 
 	Patches *testmark.PatchAccumulator
+	// Will allow unrecognized directory structures.
+	DisableStrictMode bool
 }
 
 func (tcfg *Tester) init() {
@@ -161,21 +163,22 @@ func (tcfg Tester) test(t *testing.T, data *testmark.DirEnt, allowExec, allowScr
 	sequenceHunk, sequenceMode := data.Children["sequence"]
 	scriptHunk, scriptMode := data.Children["script"]
 	if !sequenceMode && !scriptMode {
-		return
+		tcfg.skipOrFailStrictlyf(t, "dir %q does not contain a 'script' or 'sequence' hunk", data.Path)
 	}
 	if sequenceMode && scriptMode {
-		t.Logf("warning: dir %q contained both a 'script' and a 'sequence' hunk, which is nonsensical", data.Name)
-		t.SkipNow()
+		tcfg.skipOrFailStrictlyf(t, "dir %q contained both a 'script' and a 'sequence' hunk, which is nonsensical", data.Path)
 	}
 	if sequenceMode && !allowExec {
-		t.Skipf("found sequence hunk but the test framework was invoked without permission to run those")
+		tcfg.skipOrFailStrictlyf(t, "found sequence hunk but the test framework was invoked without permission to run those")
 	}
 	if scriptMode && !allowScript {
-		t.Skipf("found script hunk but the test framework was invoked without permission to run those")
+		tcfg.skipOrFailStrictlyf(t, "found script hunk but the test framework was invoked without permission to run those")
 	}
 	if *testmark.Regen && tcfg.Patches == nil {
-		t.Logf("warning: testmark.regen mode engaged, but there is no patch accumulator available here")
-		t.Skipf("nothing to do if requested to regenerate test fixtures but have nowhere to put data")
+		tcfg.skipOrFailStrictlyf(t, "%s\n%s",
+			"testmark.regen mode engaged, but there is no patch accumulator available here",
+			"nothing to do if requested to regenerate test fixtures but have nowhere to put data",
+		)
 	}
 
 	// Create a tempdir, and fill it with any files.
@@ -250,14 +253,17 @@ func (tcfg Tester) test(t *testing.T, data *testmark.DirEnt, allowExec, allowScr
 	// Do the thing.
 	switch {
 	case sequenceMode:
+		t.Logf("exec: %q", sequenceHunk.Hunk.Name)
 		exitcode = tcfg.doSequence(t, sequenceHunk.Hunk, stdin, stdout, stderr)
 	case scriptMode:
+		t.Logf("exec: %q", scriptHunk.Hunk.Name)
 		exitcode = tcfg.doScript(t, scriptHunk.Hunk, stdin, stdout, stderr)
 	}
 
 	// Okay, comparisons time.
 	// Or, regen time!
 	if ent, exists := data.Children["output"]; exists {
+		// stdout buffer should be prepared to be both stdout and stderr earlier before execution.
 		bs := stdout.(*bytes.Buffer).Bytes()
 		if *testmark.Regen {
 			tcfg.Patches.AppendPatchIfBodyDiffers(*ent.Hunk, bs)
@@ -299,21 +305,49 @@ func (tcfg Tester) test(t *testing.T, data *testmark.DirEnt, allowExec, allowScr
 		}
 	})
 
-	// Look for "then-*" dirs.
-	//  If we're already failed -- make the run block, but skip it.
-	//  If we're going to procede: make a new tempdir, copy the contents, and then procede by recursing.
+	tcfg.recurse(t, data, allowExec, allowScript, dir)
+}
+
+func (tcfg Tester) recurse(t *testing.T, data *testmark.DirEnt, allowExec bool, allowScript bool, parentTmpDir string) {
+	alreadyFailed := t.Failed()
 	for _, child := range data.ChildrenList {
-		if len(child.Name) > 5 && strings.HasPrefix(child.Name, "then-") {
-			alreadyFailed := t.Failed()
-			t.Run(child.Name, func(t *testing.T) {
-				if alreadyFailed {
-					t.Skipf("parent commands failed, so while more commands are specified, testing them is not meaningful")
-				}
-				tcfg.test(t, child, allowExec, allowScript, dir)
-			})
+		if _, exists := leafNodeTable[child.Name]; exists {
+			t.Logf("%s will not recurse into special leaf node %q", t.Name(), child.Name)
+			continue
 		}
+		t.Run(child.Name, func(t *testing.T) {
+			if len(child.Name) <= 5 || !strings.HasPrefix(child.Name, "then-") {
+				tcfg.skipOrFailStrictlyf(t, "%q does not begin with %q", child.Name, "then-")
+			}
+			if alreadyFailed {
+				// This comes after the recursion test because a file structure error should still fail
+				t.Skipf("parent commands failed, so while more commands are specified, testing them is not meaningful")
+			}
+			tcfg.test(t, child, allowExec, allowScript, parentTmpDir)
+		})
 	}
+}
+
+func (tcfg Tester) skipOrFailStrictlyf(t *testing.T, format string, args ...interface{}) {
+	if format != "" || len(args) > 0 {
+		t.Logf(format, args...)
+	}
+	if tcfg.DisableStrictMode {
+		t.SkipNow()
+	}
+	t.FailNow()
+}
 
+// Hash Table of all the "special" nodes used by testexec.
+var leafNodeTable = map[string]struct{}{
+	"exitcode": {},
+	"stderr":   {},
+	"stdout":   {},
+	"output":   {},
+	"input":    {},
+	"sequence": {},
+	"script":   {},
+	"fs":       {},
 }
 
 func (tcfg Tester) doSequence(t *testing.T, hunk *testmark.Hunk, stdin io.Reader, stdout, stderr io.Writer) (exitcode int) {

testexec/testexec_test.go

@@ -1,13 +1,16 @@
 package testexec_test
 
 import (
+	"flag"
 	"testing"
 
 	"github.com/warpfork/go-testmark"
 	"github.com/warpfork/go-testmark/testexec"
 )
 
-func Test(t *testing.T) {
+var RunFailTest = flag.Bool("run-fail-test", false, "Executes the tests which are expected to fail")
+
+func TestSelfExercise(t *testing.T) {
 	filename := "selfexercise.md"
 	doc, err := testmark.ReadFile(filename)
 	if err != nil {
@@ -26,3 +29,70 @@ func Test(t *testing.T) {
 	}
 	patches.WriteFileWithPatches(doc, filename)
 }
+
+func TestInvalid(t *testing.T) {
+	if !(*RunFailTest) {
+		t.Skipf("%s requires %q flag to execute", t.Name(), "run-fail-test")
+	}
+	filename := "invalidexercise.md"
+	doc, err := testmark.ReadFile(filename)
+	if err != nil {
+		t.Fatalf("spec file parse failed?!: %s", err)
+	}
+
+	doc.BuildDirIndex()
+	patches := testmark.PatchAccumulator{}
+	for _, dir := range doc.DirEnt.ChildrenList {
+		t.Run(dir.Name, func(t *testing.T) {
+			test := testexec.Tester{
+				Patches: &patches,
+			}
+			test.TestScript(t, dir)
+		})
+	}
+	patches.WriteFileWithPatches(doc, filename)
+}
+
+func TestStrict(t *testing.T) {
+	if !(*RunFailTest) {
+		t.Skipf("%s requires %q flag to execute", t.Name(), "run-fail-test")
+	}
+	filename := "strictexercise.md"
+	doc, err := testmark.ReadFile(filename)
+	if err != nil {
+		t.Fatalf("spec file parse failed?!: %s", err)
+	}
+
+	doc.BuildDirIndex()
+	patches := testmark.PatchAccumulator{}
+	for _, dir := range doc.DirEnt.ChildrenList {
+		t.Run(dir.Name, func(t *testing.T) {
+			test := testexec.Tester{
+				Patches: &patches,
+			}
+			test.TestScript(t, dir)
+		})
+	}
+	patches.WriteFileWithPatches(doc, filename)
+}
+
+func TestStrictDisabled(t *testing.T) {
+	filename := "strictexercise.md"
+	doc, err := testmark.ReadFile(filename)
+	if err != nil {
+		t.Fatalf("spec file parse failed?!: %s", err)
+	}
+
+	doc.BuildDirIndex()
+	patches := testmark.PatchAccumulator{}
+	for _, dir := range doc.DirEnt.ChildrenList {
+		t.Run(dir.Name, func(t *testing.T) {
+			test := testexec.Tester{
+				Patches:           &patches,
+				DisableStrictMode: true,
+			}
+			test.TestScript(t, dir)
+		})
+	}
+	patches.WriteFileWithPatches(doc, filename)
+}

testmark.go

@@ -67,6 +67,9 @@ type DirEnt struct {
 	// (Note that if there's a Hunk in this DirEnt, its name may be different -- it still has the *full* path name.)
 	Name string
 
+	// The fullpath
+	Path string
+
 	// A hunk, or nil.
 	Hunk *Hunk