Merge pull request #16 from warpfork/suite-feature
commit a04c34bf0fea924053ae4a4403befc70597ee338 · parent f242a34a55 · parent 921077c4fe
· browse this tree
Eric Myhre, 2023-05-10 15:04:33 UTC
Suite feature
diff --git a/go.mod b/go.mod index dc56ee0..b15b7ab 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,5 @@ module github.com/warpfork/go-testmark go 1.16 + +require github.com/warpfork/go-fsx v0.3.0 diff --git a/go.sum b/go.sum new file mode 100644 index 0000000..063a5f9 --- /dev/null +++ b/go.sum @@ -0,0 +1,2 @@ +github.com/warpfork/go-fsx v0.3.0 h1:RGueN83R4eOc/2oZkQ58RRxQS9JIevWgvoM55oaN9tE= +github.com/warpfork/go-fsx v0.3.0/go.mod h1:oTACCMj+Zle+vgVa5SAhGAh7WksYpLgGUCKEAVc+xPg= diff --git a/suite/suite.go b/suite/suite.go new file mode 100644 index 0000000..757b95d --- /dev/null +++ b/suite/suite.go @@ -0,0 +1,256 @@ +package suite + +import ( + "fmt" + "io" + "io/fs" + "path" + "testing" + + "github.com/warpfork/go-fsx" + "github.com/warpfork/go-testmark" +) + +// NewManager constructs a suite manager with which you can use WorkWith to register +// patterns of filenames and hunknames within them with a TestingFunctor, +// and then run the whole suite together. +// +// Note that if you want automatic fixture regen features to work, +// the filesystem you hand in to this constructor must support `fsx.FSSupportingWrite`. +// (In practice, this often means you want to use `github.com/warpfork/go-fsx/osfs.DirFS` +// where you otherwise might've used stdlib `os.DirFS` to construct the filesystem reference.) +func NewManager(fs fsx.FS) *Manager { + return &Manager{ + fs: fs, + workset: make(map[string]fileContentExpectations), + } +} + +// Using suite.Manager to run tests is not strictly necessary, +// but saves a lot of boilerplate, offers a lot of features and guardrails, +// and makes your test setup more declarative and easy to read: +// +// - suite.Manager lets you specify groups of files to treat as test data. +// - suite.Manager lets you associate TestingFunctor callbacks with hunk names by globbing. +// - suite.Manager automatically names your tests based on the filename and hunk names. +// - suite.Manager automatically rigs up fixture regeneration for you when `-testmark.regen=true`. +// - suite.Manager will warn you about any hunks that go unused in a file (helps detect typos!). +// - suite.Manager will warn you about any hunk globs that go unmatched in a file. +// +// In general, using suite.Manager will help make sure your fixture files and test cases +// stay aligned as the code evolves: if you remove tests that used some hunks, you'll get notified; +// if you add tests that expect certain patterns of hunks in some files, but didn't write them yet, +// you'll get notified; etc. +// +// Approximately the only thing suite.Manager can't help you with is if you use whole files for organization, +// and you remove some files (or add some features or tests that would expect whole new files), +// and you're using globbing to register filenames, then suite.Manager can't help you infer what's missing. +type Manager struct { + fs fsx.FS + + workset map[string]fileContentExpectations +} + +type fileContentExpectations struct { + filename string + + handlers map[HunkGlob]TestingFunctor +} + +// TestingFunctor is the interface that test code should implement to become +// usable by suite.Manager as a handler. +// +// Examples of TestingFunctor that are well-known and easy to use immediately +// include the one produced by `testexec.NewSuiteTester`. +// +// When implementing this interface: +// The Run method will do the majority of the work. +// Other methods on the interface let the suite.Manager know how recursion should work, +// and provide some information that may be used in logging. +// +// Note that it is not necessary to import this package in order to implement this interface. +// Only symbols from testmark's core package and the go standard library are used. +type TestingFunctor interface { + Run( + 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(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. + + Name() string // Purely for diagnostic purposes. + + OwnsAllChildren() bool // If true, the suite manager will not look at any hunks beneath this subject or attempt to match other globs and testing patterns to them. +} + +type FilenameGlob string +type HunkGlob string + +// WorkWith registers one or more files to be handled with some TestingFunctor. +// A globbing pattern can be used for both the filenames, and for what hunk labels +// within the file should be handled. +// +// WorkWith can be called more than one time with the same filename, or with filename +// globs that cause the same files to be matched more than once. +// In this case, the mapping of filenames and hunk globs to TestingFunctor simply +// continue to accumulate. +// +// Both filename and hunk label globbing are per `path.Match`. +// Note that using a "glob" that's a literal, with no actual pattern matching markers, is acceptable. +// +// The filename glob is matched against the suite.Manager's filesystem immediately. +// If it matches no files, an error is returned. +// (Use MustWorkWith to panic instead.) +// +// If the hunk name glob doesn't compile as a globbing pattern, an error is returned. +// (Use MustWorkWith to panic instead.) +// +// If many testing patterns are registered, and the exact same hunk glob is associated with +// the same filename, the last call of WorkWith overrides the earlier one for that pattern. +// However, if distinct hunk globs later simply happen to match the same hunk label, +// then both will be called, and the order in which they are called is unspecified. +func (sm *Manager) WorkWith(files FilenameGlob, hunks HunkGlob, action TestingFunctor) error { + _, err := path.Match(string(hunks), "") + if err != nil { + return fmt.Errorf("hunk label glob does not compile: %w", err) + } + matches, err := fs.Glob(sm.fs, string(files)) + if err != nil { + return fmt.Errorf("filename glob does not compile: %w", err) + } + if len(matches) == 0 { + return fmt.Errorf("filename glob %q matched no files", files) + } + for _, match := range matches { + if _, exists := sm.workset[match]; !exists { + sm.workset[match] = fileContentExpectations{ + filename: match, + handlers: make(map[HunkGlob]TestingFunctor), + } + } + sm.workset[match].handlers[hunks] = action + } + return nil +} + +// MustWorkWith is exactly as per WorkWith, but panics in case of errors. +func (sm *Manager) MustWorkWith(files FilenameGlob, hunks HunkGlob, action TestingFunctor) { + if err := sm.WorkWith(files, hunks, action); err != nil { + panic(err) + } +} + +// IgnoreUnrecognized tells the suite manager that if a hunk is reported as unrecognized or unused, +// then it should be ignored instead of becoming an error. +// +// A typical value to see this called with might be `"*/comment"`, for example. +// +// Such a setup would allow you to put comment nodes even deep inside hunk trees that are used +// by other test patterns (such as e.g. `testexec.TestPattern`) which would otherwise report them as an error. +// func (sm *Manager) IgnoreUnrecognized(files FilenameGlob, pattern HunkGlob) { +// panic("not yet implemented") // TODO +// } + +// Run launches the test suite. +// WorkWith should have been called to populate the suite before this. +// +// Subtests will be created with `t.Run` for each file, +// and another nested subtest for each hunk that matches a pattern the suite works with +// and thus causes a TestingFunctor to be invoked. +// (The TestingFunctor gets full control of `t` thereafter, and may +// create even further additional subtests.) +// +// By default, each file that the suite works with will be handled in parallel, +// and within each file, the hunks are handled in the order they appear. +// +// Calling Run more than one time is nonsensical. +func (sm *Manager) Run(t *testing.T) { + for filename, fileContentExpectations := range sm.workset { + filename := filename + fileContentExpectations := fileContentExpectations + t.Run(filename, func(t *testing.T) { + t.Parallel() + + // Open the file. + f, err := sm.fs.Open(filename) + if err != nil { + t.Fatalf("could not load testmark file: %s", err) + } + + // Parse the file. + tmDoc, err := testmark.Read(f) + f.Close() + if err != nil { + t.Fatalf("could not parse testmark file %q: %s", filename, err) + } + tmDoc.BuildDirIndex() + + // Prepare to write back patches, if appropriate. + var patchAccum *testmark.PatchAccumulator + if *testmark.Regen { + patchAccum = &testmark.PatchAccumulator{} + defer func() { + f, err := fsx.OpenFile(sm.fs, filename, fsx.O_TRUNC|fsx.O_WRONLY, 0777) + if err != nil { + t.Fatalf("could not open file to write regenerated fixture: %s", err) + } + patchAccum.WriteWithPatches(tmDoc, f.(io.Writer)) + f.Close() + }() + } + + // Before we begin walking, prepare to remember which things are usedHunks... or explicitly flagged as unknown. + usedHunks := map[string]struct{}{} + unrecognizedHunks := map[string]string{} + usedGlobs := map[HunkGlob]struct{}{} + reportUse := func(hunkName string) { usedHunks[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 functor's Run method. + for _, ent := range tmDoc.DirEnt.ChildrenList { + // Check every handler associated with this filename if it matches this hunk. + for hunkGlob, action := range fileContentExpectations.handlers { + if match, _ := path.Match(string(hunkGlob), ent.Path); match { + usedGlobs[hunkGlob] = struct{}{} + t.Run(ent.Path, func(t *testing.T) { + err := action.Run(t, filename, ent, reportUse, reportUnrecog, patchAccum) + if err != nil { + t.Fatalf("error while running the %s testing pattern on hunk %q in file %q: %s", action.Name(), ent.Path, filename, err) + } + }) + } + } + } + + // Make another subtest to report any unused or explicitly unrecognized hunks. + // Only do this if there are any of them; otherwise, don't bother cluttering up the reports. + if len(tmDoc.HunksByName) == 0 || + len(usedHunks) < len(tmDoc.HunksByName) || + len(unrecognizedHunks) > 0 || + len(usedGlobs) < len(fileContentExpectations.handlers) { + t.Run("dangling references", func(t *testing.T) { + if len(tmDoc.HunksByName) == 0 { + t.Errorf("file %q contained no testmark hunks at all and caused no tests to be exercised in this suite", filename) + } + for hunkName, _ := range tmDoc.HunksByName { + if _, exists := usedHunks[string(hunkName)]; !exists { + t.Errorf("hunk label %q in file %q was not used by any 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 { + t.Errorf("the glob %q matched zero hunk labels in file %q and caused no tests to be exercised in this suite", hunkGlob, filename) + } + } + }) + } + }) + + } +} 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..8bc3b10 --- /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 SuiteTester type matches the suite.TestingFunctor interface. +// By doing this in a "_test.go" file, we avoid importing the suite package in the testexec package. +var _ suite.TestingFunctor = testexec.SuiteTester{} + +func TestSuiteMode(t *testing.T) { + t.Run("selfexericse file", func(t *testing.T) { + sm := suite.NewManager(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.NewManager(osfs.DirFS(".")) + sm.MustWorkWith("strictexercise.md", "*", testexec.NewSuiteTester(testexec.Tester{})) + sm.Run(t) + }) +}