Use Suite feature in testexec. optionally.
commit 04a9a8ce6c613db918bdbb0279d11e5d7e7ca41b · parent 301b8bd3ac
· browse this tree
Eric Myhre, 2023-05-10 12:08:42 UTC
This replaces the strict mode feature. If you use the suite style, you get strict mode. (We could re-introduce a piece of config that causes testexec to claim use for all child hunks, or at least to decline to report any explicitly unrecognized hunks, but today, I don't feel inclined. I don't think use of that feature has been above about zero.) Removed many logf statements. Successful tests should be quiet. Switched quite a few errors that were previously conditional back to immediate fatalf, where they should be. Others disappeared into becoming part of the suite report hook system. For the testing of this thing itself: I don't love the `RunFailTest` thing we've got going on here, but I see why it's a stand-in for mocking the entirety of `*testing.T`, and I'm not gonna fight with that today. If you do run the new test with that flag (or just the condition commented out), you'll indeed see nice errors from the suite system about unused and unrecognized hunks.
diff --git a/suite/suite.go b/suite/suite.go index c847d06..392fdd8 100644 --- a/suite/suite.go +++ b/suite/suite.go @@ -75,8 +75,8 @@ type TestingPattern interface { t *testing.T, // The standard testing object, for obvious purposes. filename string, // The filename the subject data was loaded from. Typically not needed (data comes in via `subject` and, if appropriate, can flow out through `patchAccum`). subject *testmark.DirEnt, // The subject hunk (and enclosing dirent, in case you want to navigate to child hunks). - reportUse func(string), // Should be called with the full path of any hunk that's consumed by this test. Used to detect orphaned hunks that went unused by the whole suite. - reportUnrecog func(string), // If this test code owns all child hunks, it may call this to report one that it doesn't recognize. + reportUse func(hunkPath string), // Should be called with the full path of any hunk that's consumed by this test. Used to detect orphaned hunks that went unused by the whole suite. + reportUnrecog func(hunkPath string, reason string), // If this test code owns all child hunks, it may call this to report one that it doesn't recognize. patchAccum *testmark.PatchAccumulator, // If non-nil, means regenerating golden master data is requested instead of testing. ) error // Run may return errors or call t.Fatal itself. @@ -203,10 +203,10 @@ func (sm *SuiteManager) Run(t *testing.T) { // Before we begin walking, prepare to remember which things are usedHunks... or explicitly flagged as unknown. usedHunks := map[string]struct{}{} - unrecognizedHunks := map[string]struct{}{} + unrecognizedHunks := map[string]string{} usedGlobs := map[HunkGlob]struct{}{} reportUse := func(hunkName string) { usedHunks[hunkName] = struct{}{} } - reportUnrecog := func(hunkName string) { unrecognizedHunks[hunkName] = struct{}{} } + reportUnrecog := func(hunkName string, reason string) { unrecognizedHunks[hunkName] = reason } // Range over the hunks, treating labels as if they're filesystem paths (e.g., "/" groups them). // For every match, create a subtest with the hunk path as a name, and call the test pattern. @@ -240,8 +240,8 @@ func (sm *SuiteManager) Run(t *testing.T) { t.Errorf("hunk label %q in file %q was not used by any tests in this suite", hunkName, filename) } } - for _, hunkName := range unrecognizedHunks { - t.Errorf("hunk label %q in file %q was flagged as unrecognized by one of the tests in this suite", hunkName, filename) + for hunkName, reason := range unrecognizedHunks { + t.Errorf("hunk label %q in file %q was flagged as unrecognized by one of the tests in this suite -- reason: %s", hunkName, filename, reason) } for hunkGlob, _ := range fileContentExpectations.handlers { if _, exists := usedGlobs[hunkGlob]; !exists { diff --git a/testexec/testexec.go b/testexec/testexec.go index cf19f17..28b5a97 100644 --- a/testexec/testexec.go +++ b/testexec/testexec.go @@ -60,7 +60,9 @@ type AssertFn func(t *testing.T, actual, expect string) // // The 'Patches' accumulator will be used to gather new fixture data if `testmark.Regen` is true. // (If the pointer is nil, a warning will be logged.) -// It is the user's responsibility to actually apply the patches and save the updated document. +// If the NewSuiteTester constructor is used, `Patches` will be wired and handled automatically. +// Otherwise, if `Patches` is handled manually, it becomes the user's responsibility +// to actually apply the patches and save the updated document. type Tester struct { ExecFn ScriptFn @@ -68,8 +70,9 @@ type Tester struct { AssertFn Patches *testmark.PatchAccumulator - // Will allow unrecognized directory structures. - DisableStrictMode bool + + reportUse func(string) // Used to wire with suite, if you use NewSuiteTester. + reportUnrecog func(string, string) // Used to wire with suite, if you use NewSuiteTester. } func (tcfg *Tester) init() { @@ -82,6 +85,12 @@ func (tcfg *Tester) init() { if tcfg.AssertFn == nil { tcfg.AssertFn = defaultAssertFn } + if tcfg.reportUse == nil { + tcfg.reportUse = func(string) {} + } + if tcfg.reportUnrecog == nil { + tcfg.reportUnrecog = func(string, string) {} + } } // TestSequence runs a test based on a "sequence" instruction -- a hunk called "sequence" should have a series of lines, @@ -163,19 +172,19 @@ 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 { - tcfg.skipOrFailStrictlyf(t, "dir %q does not contain a 'script' or 'sequence' hunk", data.Path) + t.Fatalf("dir %q does not contain a 'script' or 'sequence' hunk", data.Path) } if sequenceMode && scriptMode { - tcfg.skipOrFailStrictlyf(t, "dir %q contained both a 'script' and a 'sequence' hunk, which is nonsensical", data.Path) + t.Fatalf("dir %q contained both a 'script' and a 'sequence' hunk, which is nonsensical", data.Path) } if sequenceMode && !allowExec { - tcfg.skipOrFailStrictlyf(t, "found sequence hunk but the test framework was invoked without permission to run those") + t.Fatalf("found sequence hunk but the test framework was invoked without permission to run those") } if scriptMode && !allowScript { - tcfg.skipOrFailStrictlyf(t, "found script hunk but the test framework was invoked without permission to run those") + t.Fatalf("found script hunk but the test framework was invoked without permission to run those") } if *testmark.Regen && tcfg.Patches == nil { - tcfg.skipOrFailStrictlyf(t, "%s\n%s", + t.Fatalf("%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", ) @@ -194,30 +203,30 @@ func (tcfg Tester) test(t *testing.T, data *testmark.DirEnt, allowExec, allowScr var err error dir, err = ioutil.TempDir("", "testmarkexec") if err != nil { - t.Errorf("test aborted: could not create tempdir: %s", err) + t.Fatalf("test aborted: could not create tempdir: %s", err) } defer os.RemoveAll(dir) retreat, err := os.Getwd() if err != nil { - t.Errorf("test aborted: could not find cwd: %s", err) + t.Fatalf("test aborted: could not find cwd: %s", err) } defer os.Chdir(retreat) if err := os.Chdir(dir); err != nil { - t.Errorf("test aborted: could not chdir to tempdir: %s", err) + t.Fatalf("test aborted: could not chdir to tempdir: %s", err) } // If there was a parent tempdir: copy those files first. if parentTmpdir != "" { if err := copyFiles(parentTmpdir, dir); err != nil { - t.Errorf("test aborted: could not populate files to tempdir: %s", err) + t.Fatalf("test aborted: could not populate files to tempdir: %s", err) } } // Create any new files. fsEnt := data.Children["fs"] if fsEnt != nil { - if err := createFiles(fsEnt, "."); err != nil { - t.Errorf("test aborted: could not populate files to tempdir: %s", err) + if err := tcfg.createFiles(fsEnt, "."); err != nil { + t.Fatalf("test aborted: could not populate files to tempdir: %s", err) } } } @@ -225,6 +234,7 @@ func (tcfg Tester) test(t *testing.T, data *testmark.DirEnt, allowExec, allowScr // Prepare output buffers. var stdout, stderr io.Writer if _, exists := data.Children["output"]; exists { + tcfg.reportUse(data.Children["output"].Path) stdout = &bytes.Buffer{} stderr = stdout if _, exists := data.Children["stdout"]; exists { @@ -235,9 +245,11 @@ func (tcfg Tester) test(t *testing.T, data *testmark.DirEnt, allowExec, allowScr } } if _, exists := data.Children["stdout"]; exists { + tcfg.reportUse(data.Children["stdout"].Path) stdout = &bytes.Buffer{} } if _, exists := data.Children["stderr"]; exists { + tcfg.reportUse(data.Children["stderr"].Path) stderr = &bytes.Buffer{} } var exitcode int @@ -245,6 +257,7 @@ func (tcfg Tester) test(t *testing.T, data *testmark.DirEnt, allowExec, allowScr // Prepare an input buffer, if applicable. var stdin io.Reader if ent := data.Children["input"]; ent != nil { + tcfg.reportUse(data.Children["input"].Path) stdin = bytes.NewReader(ent.Hunk.Body) } else { stdin = bytes.NewReader(nil) @@ -253,10 +266,10 @@ 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) + tcfg.reportUse(sequenceHunk.Path) exitcode = tcfg.doSequence(t, sequenceHunk.Hunk, stdin, stdout, stderr) case scriptMode: - t.Logf("exec: %q", scriptHunk.Hunk.Name) + tcfg.reportUse(scriptHunk.Path) exitcode = tcfg.doScript(t, scriptHunk.Hunk, stdin, stdout, stderr) } @@ -295,6 +308,7 @@ func (tcfg Tester) test(t *testing.T, data *testmark.DirEnt, allowExec, allowScr } t.Run("check-exitcode", func(t *testing.T) { if ent, exists := data.Children["exitcode"]; exists { + tcfg.reportUse(data.Children["exitcode"].Path) if *testmark.Regen { tcfg.Patches.AppendPatchIfBodyDiffers(*ent.Hunk, []byte(strconv.Itoa(exitcode))) } else { @@ -312,12 +326,12 @@ func (tcfg Tester) recurse(t *testing.T, data *testmark.DirEnt, allowExec bool, alreadyFailed := t.Failed() for _, child := range data.ChildrenList { if _, exists := leafNodeTable[child.Name]; exists { - t.Logf("%s will not recurse into special leaf node %q", t.Name(), child.Name) + //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-") + tcfg.reportUnrecog(child.Path, "a child test in testmark must be named beginning with \"then-\"") } if alreadyFailed { // This comes after the recursion test because a file structure error should still fail @@ -328,16 +342,6 @@ func (tcfg Tester) recurse(t *testing.T, data *testmark.DirEnt, allowExec bool, } } -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": {}, @@ -384,7 +388,8 @@ func (tcfg Tester) doScript(t *testing.T, hunk *testmark.Hunk, stdin io.Reader, // createFiles makes files and directories matching testmark hunks. // It creates them relative to the os cwd plus prefix -- use with care. -func createFiles(dir *testmark.DirEnt, prefix string) error { +func (tcfg Tester) createFiles(dir *testmark.DirEnt, prefix string) error { + tcfg.reportUse(dir.Path) if dir.Hunk != nil { return ioutil.WriteFile(prefix, dir.Hunk.Body, 0644) } else { @@ -393,7 +398,7 @@ func createFiles(dir *testmark.DirEnt, prefix string) error { } } for _, ent := range dir.Children { - if err := createFiles(ent, filepath.Join(prefix, ent.Name)); err != nil { + if err := tcfg.createFiles(ent, filepath.Join(prefix, ent.Name)); err != nil { return err } } diff --git a/testexec/testexec_test.go b/testexec/testexec_test.go index c712e2e..ca339c2 100644 --- a/testexec/testexec_test.go +++ b/testexec/testexec_test.go @@ -75,24 +75,3 @@ func TestStrict(t *testing.T) { } 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) -} diff --git a/testexec/testpattern.go b/testexec/testpattern.go new file mode 100644 index 0000000..f2e4ebe --- /dev/null +++ b/testexec/testpattern.go @@ -0,0 +1,32 @@ +package testexec + +import ( + "testing" + + "github.com/warpfork/go-testmark" +) + +func NewSuiteTester(tcfg Tester) SuiteTester { + return SuiteTester{tcfg} +} + +type SuiteTester struct { + tcfg Tester +} + +func (st SuiteTester) Name() string { return "testexec" } +func (st SuiteTester) OwnsAllChildren() bool { return true } +func (st SuiteTester) Run( + t *testing.T, + filename string, + subject *testmark.DirEnt, + reportUse func(string), + reportUnrecog func(string, string), + patchAccum *testmark.PatchAccumulator, +) error { + st.tcfg.reportUse = reportUse + st.tcfg.reportUnrecog = reportUnrecog + st.tcfg.Patches = patchAccum + st.tcfg.Test(t, subject) + return nil +} diff --git a/testexec/testpattern_test.go b/testexec/testpattern_test.go new file mode 100644 index 0000000..e4014e2 --- /dev/null +++ b/testexec/testpattern_test.go @@ -0,0 +1,29 @@ +package testexec_test + +import ( + "testing" + + "github.com/warpfork/go-fsx/osfs" + "github.com/warpfork/go-testmark/suite" + "github.com/warpfork/go-testmark/testexec" +) + +// Assert that our testingPattern type matches the suite.TestingPattern interface. +// By doing this in a "_test.go" file, we avoid importing the suite package in the testexec package. +var _ suite.TestingPattern = testexec.SuiteTester{} + +func TestSuiteMode(t *testing.T) { + t.Run("selfexericse file", func(t *testing.T) { + sm := suite.NewSuiteManager(osfs.DirFS(".")) + sm.MustWorkWith("selfexercise.md", "*", testexec.NewSuiteTester(testexec.Tester{})) + sm.Run(t) + }) + t.Run("strictexericse file", func(t *testing.T) { + if !(*RunFailTest) { + t.Skipf("%s requires %q flag to execute", t.Name(), "run-fail-test") + } + sm := suite.NewSuiteManager(osfs.DirFS(".")) + sm.MustWorkWith("strictexercise.md", "*", testexec.NewSuiteTester(testexec.Tester{})) + sm.Run(t) + }) +}