From a7f793e3eeaf1c8ff32b03fc95c2551dbc6f37f9 Mon Sep 17 00:00:00 2001 From: Georgios Kampitakis <50364739+gkampitakis@users.noreply.github.com> Date: Mon, 22 Jan 2024 19:36:40 +0000 Subject: [PATCH] feat: don't create multiple snapshots when -test.count>1 (#90) --- Makefile | 4 +-- snaps/clean.go | 15 ++++++--- snaps/clean_test.go | 61 +++++++++++++++++++++++++------------ snaps/matchJSON.go | 3 ++ snaps/matchJSON_test.go | 3 ++ snaps/matchSnapshot.go | 4 +++ snaps/matchSnapshot_test.go | 3 ++ snaps/snapshot.go | 25 ++++++++++----- snaps/snapshot_test.go | 24 +++++++++++++++ 9 files changed, 110 insertions(+), 32 deletions(-) diff --git a/Makefile b/Makefile index cffdb7c..7adb3f2 100644 --- a/Makefile +++ b/Makefile @@ -16,7 +16,7 @@ format: ## Format code golines . -w test: ## Run tests - go test -race -count=1 -cover ./... + go test -race -count=10 -shuffle on -cover ./... test-verbose: ## Run tests with verbose output - go test -race -count=1 -v -cover ./... + go test -race -count=10 -shuffle on -v -cover ./... diff --git a/snaps/clean.go b/snaps/clean.go index 7e15d15..9344202 100644 --- a/snaps/clean.go +++ b/snaps/clean.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "slices" + "strconv" "strings" "sync" "testing" @@ -82,12 +83,14 @@ func Clean(m *testing.M, opts ...CleanOpts) { // This is just for making sure Clean is called from TestMain _ = m runOnly := flag.Lookup("test.run").Value.String() + count, _ := strconv.Atoi(flag.Lookup("test.count").Value.String()) - obsoleteFiles, usedFiles := examineFiles(testsRegistry.values, runOnly, shouldClean && !isCI) + obsoleteFiles, usedFiles := examineFiles(testsRegistry.cleanup, runOnly, shouldClean && !isCI) obsoleteTests, err := examineSnaps( - testsRegistry.values, + testsRegistry.cleanup, usedFiles, runOnly, + count, shouldClean && !isCI, opt.Sort && !isCI, ) @@ -200,6 +203,7 @@ func examineSnaps( registry map[string]map[string]int, used []string, runOnly string, + count int, update, sort bool, ) ([]string, error) { @@ -216,7 +220,7 @@ func examineSnaps( var hasDiffs bool - registeredTests := occurrences(registry[snapPath]) + registeredTests := occurrences(registry[snapPath], count) s := snapshotScanner(f) for s.Scan() { @@ -405,9 +409,12 @@ e.g as it means there are 3 snapshots created inside TestAdd */ -func occurrences(tests map[string]int) set { +func occurrences(tests map[string]int, count int) set { result := make(set, len(tests)) for testID, counter := range tests { + // divide a test's counter by count (how many times the go test suite ran) + // this gives us how many snapshots were created in a single test run. + counter = counter / count if counter > 1 { for i := 1; i <= counter; i++ { result[fmt.Sprintf("%s - %d", testID, i)] = struct{}{} diff --git a/snaps/clean_test.go b/snaps/clean_test.go index 3318de1..35966fb 100644 --- a/snaps/clean_test.go +++ b/snaps/clean_test.go @@ -151,7 +151,7 @@ func TestExamineSnaps(t *testing.T) { filepath.FromSlash(dir2 + "/test2.snap"), } - obsolete, err := examineSnaps(tests, used, "", shouldUpdate, sort) + obsolete, err := examineSnaps(tests, used, "", 1, shouldUpdate, sort) test.Equal(t, []string{}, obsolete) test.NoError(t, err) @@ -172,7 +172,7 @@ func TestExamineSnaps(t *testing.T) { // Removing the test entirely delete(tests[used[1]], "TestDir2_2/TestSimple") - obsolete, err := examineSnaps(tests, used, "", shouldUpdate, sort) + obsolete, err := examineSnaps(tests, used, "", 1, shouldUpdate, sort) content1 := test.GetFileContent(t, used[0]) content2 := test.GetFileContent(t, used[1]) @@ -200,7 +200,7 @@ func TestExamineSnaps(t *testing.T) { delete(tests[used[0]], "TestDir1_3/TestSimple") delete(tests[used[1]], "TestDir2_1/TestSimple") - obsolete, err := examineSnaps(tests, used, "", shouldUpdate, sort) + obsolete, err := examineSnaps(tests, used, "", 1, shouldUpdate, sort) content1 := test.GetFileContent(t, used[0]) content2 := test.GetFileContent(t, used[1]) @@ -258,7 +258,7 @@ string hello world 2 2 1 filepath.FromSlash(dir2 + "/test2.snap"), } - obsolete, err := examineSnaps(tests, used, "", shouldUpdate, sort) + obsolete, err := examineSnaps(tests, used, "", 1, shouldUpdate, sort) test.NoError(t, err) test.Equal(t, 0, len(obsolete)) @@ -290,7 +290,7 @@ string hello world 2 2 1 delete(tests[used[0]], "TestDir1_3/TestSimple") delete(tests[used[1]], "TestDir2_1/TestSimple") - obsolete, err := examineSnaps(tests, used, "", shouldUpdate, sort) + obsolete, err := examineSnaps(tests, used, "", 1, shouldUpdate, sort) test.NoError(t, err) test.Equal(t, []string{ @@ -313,22 +313,45 @@ string hello world 2 2 1 } func TestOccurrences(t *testing.T) { - tests := map[string]int{ - "add": 3, - "subtract": 1, - "divide": 2, - } + t.Run("when count 1", func(t *testing.T) { + tests := map[string]int{ + "add": 3, + "subtract": 1, + "divide": 2, + } - expected := set{ - "add - 1": {}, - "add - 2": {}, - "add - 3": {}, - "subtract - 1": {}, - "divide - 1": {}, - "divide - 2": {}, - } + expected := set{ + "add - 1": {}, + "add - 2": {}, + "add - 3": {}, + "subtract - 1": {}, + "divide - 1": {}, + "divide - 2": {}, + } + + test.Equal(t, expected, occurrences(tests, 1)) + }) - test.Equal(t, expected, occurrences(tests)) + t.Run("when count 3", func(t *testing.T) { + tests := map[string]int{ + "add": 12, + "subtract": 3, + "divide": 9, + } + + expected := set{ + "add - 1": {}, + "add - 2": {}, + "add - 3": {}, + "add - 4": {}, + "subtract - 1": {}, + "divide - 1": {}, + "divide - 2": {}, + "divide - 3": {}, + } + + test.Equal(t, expected, occurrences(tests, 3)) + }) } func TestSummary(t *testing.T) { diff --git a/snaps/matchJSON.go b/snaps/matchJSON.go index db07b94..9fd5814 100644 --- a/snaps/matchJSON.go +++ b/snaps/matchJSON.go @@ -65,6 +65,9 @@ func matchJSON(c *config, t testingT, input any, matchers ...match.JSONMatcher) snapPath, snapPathRel := snapshotPath(c) testID := testsRegistry.getTestID(snapPath, t.Name()) + t.Cleanup(func() { + testsRegistry.reset(snapPath, t.Name()) + }) j, err := validateJSON(input) if err != nil { diff --git a/snaps/matchJSON_test.go b/snaps/matchJSON_test.go index 53d4cc6..6f30d93 100644 --- a/snaps/matchJSON_test.go +++ b/snaps/matchJSON_test.go @@ -59,6 +59,9 @@ func TestMatchJSON(t *testing.T) { test.Equal(t, 2, line) test.Equal(t, expected, snap) test.Equal(t, 1, testEvents.items[added]) + // clean up function called + test.Equal(t, 0, testsRegistry.running[snapPath]["mock-name"]) + test.Equal(t, 1, testsRegistry.cleanup[snapPath]["mock-name"]) }) } }) diff --git a/snaps/matchSnapshot.go b/snaps/matchSnapshot.go index a410d8c..b381e68 100644 --- a/snaps/matchSnapshot.go +++ b/snaps/matchSnapshot.go @@ -56,6 +56,10 @@ func matchSnapshot(c *config, t testingT, values ...any) { snapPath, snapPathRel := snapshotPath(c) testID := testsRegistry.getTestID(snapPath, t.Name()) + t.Cleanup(func() { + testsRegistry.reset(snapPath, t.Name()) + }) + snapshot := takeSnapshot(values) prevSnapshot, line, err := getPrevSnapshot(testID, snapPath) if errors.Is(err, errSnapNotFound) { diff --git a/snaps/matchSnapshot_test.go b/snaps/matchSnapshot_test.go index 4877a69..186fa30 100644 --- a/snaps/matchSnapshot_test.go +++ b/snaps/matchSnapshot_test.go @@ -73,6 +73,9 @@ func TestMatchSnapshot(t *testing.T) { test.GetFileContent(t, snapPath), ) test.Equal(t, 1, testEvents.items[added]) + // clean up function called + test.Equal(t, 0, testsRegistry.running[snapPath]["mock-name"]) + test.Equal(t, 1, testsRegistry.cleanup[snapPath]["mock-name"]) }) t.Run("if it's running on ci should skip", func(t *testing.T) { diff --git a/snaps/snapshot.go b/snaps/snapshot.go index 1a17bb4..03115af 100644 --- a/snaps/snapshot.go +++ b/snaps/snapshot.go @@ -86,7 +86,8 @@ This also helps with keeping track with obsolete snaps map[snap path]: map[testname]: */ type syncRegistry struct { - values map[string]map[string]int + running map[string]map[string]int + cleanup map[string]map[string]int sync.Mutex } @@ -95,21 +96,31 @@ type syncRegistry struct { func (s *syncRegistry) getTestID(snapPath, testName string) string { s.Lock() - if _, exists := s.values[snapPath]; !exists { - s.values[snapPath] = make(map[string]int) + if _, exists := s.running[snapPath]; !exists { + s.running[snapPath] = make(map[string]int) + s.cleanup[snapPath] = make(map[string]int) } - s.values[snapPath][testName]++ - c := s.values[snapPath][testName] + s.running[snapPath][testName]++ + s.cleanup[snapPath][testName]++ + c := s.running[snapPath][testName] s.Unlock() return fmt.Sprintf("[%s - %d]", testName, c) } +// reset sets only the number of running registry for the given test to 0. +func (s *syncRegistry) reset(snapPath, testName string) { + s.Lock() + s.running[snapPath][testName] = 0 + s.Unlock() +} + func newRegistry() *syncRegistry { return &syncRegistry{ - values: make(map[string]map[string]int), - Mutex: sync.Mutex{}, + running: make(map[string]map[string]int), + cleanup: make(map[string]map[string]int), + Mutex: sync.Mutex{}, } } diff --git a/snaps/snapshot_test.go b/snaps/snapshot_test.go index c45610d..cacbc56 100644 --- a/snaps/snapshot_test.go +++ b/snaps/snapshot_test.go @@ -26,6 +26,30 @@ func TestSyncRegistry(t *testing.T) { test.Equal(t, "[test - 6]", registry.getTestID("/file", "test")) test.Equal(t, "[test-v2 - 1]", registry.getTestID("/file", "test-v2")) + test.Equal(t, registry.cleanup, registry.running) + }) + + t.Run("should reset running registry", func(t *testing.T) { + wg := sync.WaitGroup{} + registry := newRegistry() + + for i := 0; i < 100; i++ { + wg.Add(1) + + go func() { + registry.getTestID("/file", "test") + wg.Done() + }() + } + + wg.Wait() + + registry.reset("/file", "test") + + // running registry start from 0 again + test.Equal(t, "[test - 1]", registry.getTestID("/file", "test")) + // cleanup registry still has 101 + test.Equal(t, 101, registry.cleanup["/file"]["test"]) }) }