Add strict structure enforcement
commit 59d311ae523903d341431ab6597b8f32a49c028e · parent 4e855bce0a
· browse this tree
Calvin Behling, 2022-11-16 17:50:23 UTC
- Removes the recursion override. - Adds strict default mode - Adds tests for failing conditions that need to be run manually because there's really just not a good way to do that. - Splits test exercise markdown files into self, strict, and invalid based on whether they should pass or fail always or in strict mode.
diff --git a/go.mod b/go.mod index 071e0b4..dc56ee0 100644 --- a/go.mod +++ b/go.mod @@ -1,5 +1,3 @@ module github.com/warpfork/go-testmark go 1.16 - -require github.com/frankban/quicktest v1.14.3 // indirect diff --git a/index.go b/index.go index 5783e43..6c56b29 100644 --- a/index.go +++ b/index.go @@ -34,10 +34,10 @@ func (dirent *DirEnt) fill(pathSegs []string, pathIdx int, hunk Hunk) { } l := len(dirent.ChildrenList) - dirent.ChildrenList = append(dirent.ChildrenList, DirEnt{ + dirent.ChildrenList = append(dirent.ChildrenList, &DirEnt{ Name: pathSegs[pathIdx], Path: strings.Join(pathSegs[:pathIdx+1], "/"), }) - dirent.Children[pathSegs[pathIdx]] = &dirent.ChildrenList[l] + dirent.Children[pathSegs[pathIdx]] = dirent.ChildrenList[l] dirent.ChildrenList[l].fill(pathSegs, pathIdx+1, hunk) } diff --git a/testexec/defaults.go b/testexec/defaults.go index 2403593..3ecad65 100644 --- a/testexec/defaults.go +++ b/testexec/defaults.go @@ -1,14 +1,10 @@ package testexec import ( - "fmt" "io" "os/exec" - "strings" "syscall" "testing" - - "github.com/warpfork/go-testmark" ) func ExecFn_Exec(args []string, stdin io.Reader, stdout, stderr io.Writer) (exitcode int, oshit error) { @@ -50,28 +46,3 @@ func defaultAssertFn(t *testing.T, actual, expect string) { t.Errorf("expected: %q; actual: %q", expect, actual) } } - -// Hash Table of all the "special" nodes used by testmark. -var DefaultLeafNodes = map[string]struct{}{ - "exitcode": {}, - "stderr": {}, - "stdout": {}, - "output": {}, - "input": {}, - "sequence": {}, - "script": {}, - "fs": {}, -} - -func RecursionFn_Then(t *testing.T, dir testmark.DirEnt) error { - t.Helper() - if _, exists := DefaultLeafNodes[dir.Name]; exists { - t.Logf("will not recurse into special leaf node %q", dir.Name) - return IgnoreRecursion - } - if len(dir.Name) > 5 && strings.HasPrefix(dir.Name, "then-") { - return nil - } - t.Logf("skipping dir recursion: %q does not begin with %q ", dir.Name, "then-") - return fmt.Errorf("%q does not begin with %q: %w", dir.Name, "then-", SkipRecursion) -} diff --git a/testexec/invalidexercise.md b/testexec/invalidexercise.md new file mode 100644 index 0000000..b8440e6 --- /dev/null +++ b/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) +``` +``` diff --git a/testexec/selfexercise.md b/testexec/selfexercise.md index 1040c6c..0052615 100644 --- a/testexec/selfexercise.md +++ b/testexec/selfexercise.md @@ -100,22 +100,3 @@ cat - | sed 's/ is/ was/' | sed s/will/should/ this was stdin and should be echoed ``` ---- - -The default recursion function won't reach everything if you don't formulate your paths correctly. - -[testmark]:# (bad/script) -``` - # no-op -``` - -[testmark]:# (bad/then-missing-script/then-another-thing/script) -``` - # no-op, this will not be reached because it missing executable blocks in parent steps -``` - -[testmark]:# (bad/not-a-then-statement/script) -``` - # no-op, this script will not run because the subdirectory does not begin with "then-" -``` - diff --git a/testexec/strictexercise.md b/testexec/strictexercise.md new file mode 100644 index 0000000..5182591 --- /dev/null +++ b/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 +``` + diff --git a/testexec/testexec.go b/testexec/testexec.go index 9c2e30d..6b15b43 100644 --- a/testexec/testexec.go +++ b/testexec/testexec.go @@ -10,7 +10,6 @@ package testexec import ( "bytes" - "errors" "io" "io/ioutil" "os" @@ -51,18 +50,6 @@ type FilterFn func(line string) (replacement string) // The default behavior, if a Tester object doesn't get an AssertFn, is to fall back to a very basic call to `t.Errorf`. type AssertFn func(t *testing.T, actual, expect string) -var ( - // SkipRecursion can be returned by a RecursionFn for paths that should show up in the test as skipped - SkipRecursion = errors.New("skip this directory") - // IgnoreRecursion can be returned by a RecursionFn for paths that should not run a new test at all - IgnoreRecursion = errors.New("ignore this directory") -) - -// RecursionFn can be used in Tester to specify a recursion pattern -// The default recursion pattern will recurse whenever it finds "then-" -// Returning a non-nil error will cause the test for the directory to fail unless the error is a SkipRecursion or IgnoreRecursion error -type RecursionFn func(*testing.T, testmark.DirEnt) error - // Tester is a configuration-gathering structure. // Each of the `Test*` methods upon it will use these callbacks to define their behavior. // @@ -79,9 +66,10 @@ type Tester struct { ScriptFn FilterFn AssertFn - RecursionFn Patches *testmark.PatchAccumulator + // Will allow unrecognized directory structures. + DisableStrictMode bool } func (tcfg *Tester) init() { @@ -175,20 +163,40 @@ 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 { - t.Skipf("dir %q does not contain a 'script' or 'sequence' hunk and will be skipped", data.Name) + t.Logf("dir %q does not contain a 'script' or 'sequence' hunk", data.Path) + if tcfg.DisableStrictMode { + t.SkipNow() + } + t.FailNow() } if sequenceMode && scriptMode { - t.Skipf("warning: dir %q contained both a 'script' and a 'sequence' hunk, which is nonsensical", data.Name) + t.Logf("dir %q contained both a 'script' and a 'sequence' hunk, which is nonsensical", data.Path) + if tcfg.DisableStrictMode { + t.SkipNow() + } + t.FailNow() } if sequenceMode && !allowExec { - t.Skipf("found sequence hunk but the test framework was invoked without permission to run those") + t.Log("found sequence hunk but the test framework was invoked without permission to run those") + if tcfg.DisableStrictMode { + t.SkipNow() + } + t.FailNow() } if scriptMode && !allowScript { - t.Skipf("found script hunk but the test framework was invoked without permission to run those") + t.Log("found script hunk but the test framework was invoked without permission to run those") + if tcfg.DisableStrictMode { + t.SkipNow() + } + t.FailNow() } 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") + t.Logf("testmark.regen mode engaged, but there is no patch accumulator available here") + t.Logf("nothing to do if requested to regenerate test fixtures but have nowhere to put data") + if tcfg.DisableStrictMode { + t.SkipNow() + } + t.FailNow() } // Create a tempdir, and fill it with any files. @@ -263,14 +271,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) @@ -315,34 +326,44 @@ func (tcfg Tester) test(t *testing.T, data *testmark.DirEnt, allowExec, allowScr tcfg.recurse(t, data, allowExec, allowScript, dir) } -func (tcfg Tester) recurse(t *testing.T, data testmark.DirEnt, allowExec bool, allowScript bool, parentTmpDir string) { - if tcfg.RecursionFn == nil { - tcfg.RecursionFn = RecursionFn_Then - } +func (tcfg Tester) recurse(t *testing.T, data *testmark.DirEnt, allowExec bool, allowScript bool, parentTmpDir string) { + alreadyFailed := t.Failed() for _, child := range data.ChildrenList { - recurseErr := tcfg.RecursionFn(t, child) - if errors.Is(recurseErr, IgnoreRecursion) { + if _, exists := leafNodeTable[child.Name]; exists { + t.Logf("%s will not recurse into special leaf node %q", t.Name(), child.Name) continue } - alreadyFailed := t.Failed() t.Run(child.Name, func(t *testing.T) { + if len(child.Name) <= 5 || !strings.HasPrefix(child.Name, "then-") { + t.Logf("%q does not begin with %q", child.Name, "then-") + if tcfg.DisableStrictMode { + t.SkipNow() + } + t.FailNow() + } 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") } - if errors.Is(recurseErr, SkipRecursion) { - t.Skip(recurseErr) - } - if recurseErr != nil { - t.Fatal(recurseErr) - } tcfg.test(t, child, allowExec, allowScript, parentTmpDir) }) } } +// 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) { t.Helper() - t.Logf("running sequence: %q", hunk.Name) // Loop over the lines in the sequence. lines := bytes.Split(hunk.Body, []byte{'\n'}) for _, line := range lines { @@ -365,7 +386,6 @@ func (tcfg Tester) doSequence(t *testing.T, hunk *testmark.Hunk, stdin io.Reader func (tcfg Tester) doScript(t *testing.T, hunk *testmark.Hunk, stdin io.Reader, stdout, stderr io.Writer) (exitcode int) { t.Helper() - t.Logf("running script: %q", hunk.Name) var err error exitcode, err = tcfg.ScriptFn(string(hunk.Body), stdin, stdout, stderr) if err != nil { diff --git a/testexec/testexec_test.go b/testexec/testexec_test.go index ae1ed70..c712e2e 100644 --- a/testexec/testexec_test.go +++ b/testexec/testexec_test.go @@ -1,16 +1,16 @@ package testexec_test import ( - "sort" + "flag" "testing" "github.com/warpfork/go-testmark" "github.com/warpfork/go-testmark/testexec" - - qt "github.com/frankban/quicktest" ) -func TestSelf(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 { @@ -30,125 +30,69 @@ func TestSelf(t *testing.T) { patches.WriteFileWithPatches(doc, filename) } -// TestRecursion tests edge cases around which nodes are traversed via recursion -func TestRecursion(t *testing.T) { - rtested := map[string]int{} - rcalled := map[string]int{} - sum := func(m map[string]int) int { - result := 0 - for _, v := range m { - result += v - } - return result +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) } - rfn := func(t *testing.T, dir testmark.DirEnt) error { - rtested[dir.Path]++ - result := testexec.RecursionFn_Then(t, dir) - if result == nil { - rcalled[dir.Path]++ - } - return result + 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) +} - filename := "selfexercise.md" +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 { - rtested[dir.Path]++ t.Run(dir.Name, func(t *testing.T) { - rcalled[dir.Path]++ test := testexec.Tester{ - Patches: &patches, - RecursionFn: rfn, + Patches: &patches, } - test.Test(t, dir) + test.TestScript(t, dir) }) } patches.WriteFileWithPatches(doc, filename) +} - pathList := collectPaths(*doc.DirEnt) - sort.Strings(pathList) - t.Log("[idx]: [test count]-[recursion count]: [path]") - for idx, path := range pathList { - check := rtested[path] - recurse := rcalled[path] - t.Logf("%02d: %d-%d: %s", idx, check, recurse, path) +func TestStrictDisabled(t *testing.T) { + filename := "strictexercise.md" + doc, err := testmark.ReadFile(filename) + if err != nil { + t.Fatalf("spec file parse failed?!: %s", err) } - qt.Assert(t, rtested[""], qt.Equals, 0) - qt.Assert(t, rcalled[""], qt.Equals, 0) - qt.Assert(t, rtested["bad"], qt.Equals, 1) - qt.Assert(t, rcalled["bad"], qt.Equals, 1) - qt.Assert(t, rtested["bad/script"], qt.Equals, 1) - qt.Assert(t, rcalled["bad/script"], qt.Equals, 0) - qt.Assert(t, rtested["bad/then-missing-script"], qt.Equals, 1) - qt.Assert(t, rcalled["bad/then-missing-script"], qt.Equals, 1) - qt.Assert(t, rtested["bad/then-missing-script/then-another-thing"], qt.Equals, 0) - qt.Assert(t, rcalled["bad/then-missing-script/then-another-thing"], qt.Equals, 0) - qt.Assert(t, rtested["bad/then-missing-script/then-another-thing/script"], qt.Equals, 0) - qt.Assert(t, rcalled["bad/then-missing-script/then-another-thing/script"], qt.Equals, 0) - qt.Assert(t, rtested["bad/not-a-then-statement"], qt.Equals, 1) - qt.Assert(t, rcalled["bad/not-a-then-statement"], qt.Equals, 0) - qt.Assert(t, rtested["bad/not-a-then-statement/script"], qt.Equals, 0) - qt.Assert(t, rcalled["bad/not-a-then-statement/script"], qt.Equals, 0) - - qt.Assert(t, rtested["using-stdin"], qt.Equals, 1) - qt.Assert(t, rcalled["using-stdin"], qt.Equals, 1) - qt.Assert(t, rtested["using-stdin/input"], qt.Equals, 1) - qt.Assert(t, rcalled["using-stdin/input"], qt.Equals, 0) - qt.Assert(t, rtested["using-stdin/output"], qt.Equals, 1) - qt.Assert(t, rcalled["using-stdin/output"], qt.Equals, 0) - qt.Assert(t, rtested["using-stdin/script"], qt.Equals, 1) - qt.Assert(t, rcalled["using-stdin/script"], qt.Equals, 0) - - qt.Assert(t, rtested["whee"], qt.Equals, 1) - qt.Assert(t, rcalled["whee"], qt.Equals, 1) - qt.Assert(t, rtested["whee/fs"], qt.Equals, 1) - qt.Assert(t, rcalled["whee/fs"], qt.Equals, 0) - qt.Assert(t, rtested["whee/fs/a"], qt.Equals, 0) - qt.Assert(t, rcalled["whee/fs/a"], qt.Equals, 0) - qt.Assert(t, rtested["whee/output"], qt.Equals, 1) - qt.Assert(t, rcalled["whee/output"], qt.Equals, 0) - qt.Assert(t, rtested["whee/script"], qt.Equals, 1) - qt.Assert(t, rcalled["whee/script"], qt.Equals, 0) - qt.Assert(t, rtested["whee/then-more-files"], qt.Equals, 1) - qt.Assert(t, rcalled["whee/then-more-files"], qt.Equals, 1) - qt.Assert(t, rtested["whee/then-more-files/fs"], qt.Equals, 1) - qt.Assert(t, rcalled["whee/then-more-files/fs"], qt.Equals, 0) - qt.Assert(t, rtested["whee/then-more-files/fs/b"], qt.Equals, 0) - qt.Assert(t, rcalled["whee/then-more-files/fs/b"], qt.Equals, 0) - qt.Assert(t, rtested["whee/then-more-files/output"], qt.Equals, 1) - qt.Assert(t, rcalled["whee/then-more-files/output"], qt.Equals, 0) - qt.Assert(t, rtested["whee/then-more-files/script"], qt.Equals, 1) - qt.Assert(t, rcalled["whee/then-more-files/script"], qt.Equals, 0) - qt.Assert(t, rtested["whee/then-touching-files"], qt.Equals, 1) - qt.Assert(t, rcalled["whee/then-touching-files"], qt.Equals, 1) - qt.Assert(t, rtested["whee/then-touching-files/output"], qt.Equals, 1) - qt.Assert(t, rcalled["whee/then-touching-files/output"], qt.Equals, 0) - qt.Assert(t, rtested["whee/then-touching-files/script"], qt.Equals, 1) - qt.Assert(t, rcalled["whee/then-touching-files/script"], qt.Equals, 0) - qt.Assert(t, rtested["whee/then-touching-files/then-subtesting-again"], qt.Equals, 1) - qt.Assert(t, rcalled["whee/then-touching-files/then-subtesting-again"], qt.Equals, 1) - qt.Assert(t, rtested["whee/then-touching-files/then-subtesting-again/output"], qt.Equals, 1) - qt.Assert(t, rcalled["whee/then-touching-files/then-subtesting-again/output"], qt.Equals, 0) - qt.Assert(t, rtested["whee/then-touching-files/then-subtesting-again/script"], qt.Equals, 1) - qt.Assert(t, rcalled["whee/then-touching-files/then-subtesting-again/script"], qt.Equals, 0) - - qt.Assert(t, sum(rtested), qt.Equals, 22) - qt.Assert(t, sum(rcalled), qt.Equals, 7) -} - -func collectPaths(d testmark.DirEnt) []string { - result := []string{d.Path} - for _, c := range d.ChildrenList { - result = append(result, collectPaths(c)...) + 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) + }) } - return result + patches.WriteFileWithPatches(doc, filename) }