feat: support sort snaps on clean (#80)

* feat: support sort snaps on clean * fix: add natural sort * improve gettestid * add test for natural sort * chore: remove images
gkampitakis · Nov 8, 2023 · 8565c6c · 8565c6c
1 parent 33c92ed
commit 8565c6c
Show file tree

Hide file tree

Showing 14 changed files with 567 additions and 147 deletions.
diff --git a/README.md b/README.md
@@ -26,13 +26,14 @@
 - [Configuration](#configuration)
 - [Update Snapshots](#update-snapshots)
   - [Clean obsolete Snapshots](#clean-obsolete-snapshots)
+  - [Sort Snapshots](#sort-snapshots)
   - [Skipping Tests](#skipping-tests)
 - [Running tests on CI](#running-tests-on-ci)
 - [No Color](#no-color)
 - [Snapshots Structure](#snapshots-structure)
 - [Acknowledgments](#acknowledgments)
 - [Contributing](./contributing.md)
-- [Notes](#notes)
+- [Appendix](#appendix)
 
 ## Installation
 
@@ -271,6 +272,21 @@ func TestMain(t *testing.M) {
 
 For more information around [TestMain](https://pkg.go.dev/testing#hdr-Main).
 
+### Sort Snapshots
+
+By default `go-snaps` is appending new snaps to the file and in case of parallel tests the order is random. If you want snaps to be sorted in deterministic order you need to use `TestMain` per package 
+
+```go
+func TestMain(t *testing.M) {
+  v := t.Run()
+
+  // After all tests have run `go-snaps` will sort snapshots
+  snaps.Clean(t, snaps.CleanOpts{Sort: true})
+
+  os.Exit(v)
+}
+```
+
 ### Skipping Tests
 
 If you want to skip one test using `t.Skip`, `go-snaps` can't keep track
@@ -322,7 +338,8 @@ map[string]interface {}{
 ---
 ```
 
-> `*.snap` files are not meant to be edited manually, this might cause unexpected results.
+> [!NOTE]
+> If your snapshot data contain characters `---` at the start of a line followed by a new line, `go-snaps` will "escape" them and save them as `/-/-/-/` to differentiate them from termination characters.
 
 ## Acknowledgments
 
@@ -332,15 +349,10 @@ This library used [Jest Snapshoting](https://jestjs.io/docs/snapshot-testing) an
 - Cupaloy is a great and simple Golang snapshoting solution.
 - The [logo](https://github.com/MariaLetta/free-gophers-pack) was made by [MariaLetta](https://github.com/MariaLetta).
 
-## Notes
-
-1. ⚠️ When running a specific test file by specifying a path
-   `go test ./my_test.go`, `go-snaps` can't track the path so it will mistakenly mark snapshots as obsolete.
-
-2. The order in which tests are written might not be the same order that snapshots are saved in the file.
+## Appendix
 
-3. If your snapshot data contain the termination characters `---` at the start of a line
-   and after a new line, `go-snaps` will "escape" them and save them as `/-/-/-/`. This
-   should not cause any diff issues (false-positives).
+> [!WARNING]
+> When running a specific test file by specifying a path `go test ./my_test.go`, `go-snaps` can't track the path so it will mistakenly mark snapshots as obsolete.
 
-4. Snapshots should be treated as code. The snapshot artifact should be committed alongside code changes, and reviewed as part of your code review process
+> [!IMPORTANT]
+> Snapshots should be treated as code. The snapshot artifact should be committed alongside code changes, and reviewed as part of your code review process
diff --git a/go.mod b/go.mod
@@ -3,10 +3,11 @@ module github.com/gkampitakis/go-snaps
 go 1.21.0
 
 require (
-	github.com/gkampitakis/ciinfo v0.2.5
+	github.com/gkampitakis/ciinfo v0.3.0
 	github.com/gkampitakis/go-diff v1.3.2
 	github.com/kr/pretty v0.3.1
-	github.com/tidwall/gjson v1.16.0
+	github.com/maruel/natural v1.1.0
+	github.com/tidwall/gjson v1.17.0
 	github.com/tidwall/pretty v1.2.1
 	github.com/tidwall/sjson v1.2.5
 )

diff --git a/go.sum b/go.sum
@@ -1,19 +1,21 @@
 github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
-github.com/gkampitakis/ciinfo v0.2.5 h1:K0mac90lGguc1conc46l0YEsB7/nioWCqSnJp/6z8Eo=
-github.com/gkampitakis/ciinfo v0.2.5/go.mod h1:1NIwaOcFChN4fa/B0hEBdAb6npDlFL8Bwx4dfRLRqAo=
+github.com/gkampitakis/ciinfo v0.3.0 h1:gWZlOC2+RYYttL0hBqcoQhM7h1qNkVqvRCV1fOvpAv8=
+github.com/gkampitakis/ciinfo v0.3.0/go.mod h1:1NIwaOcFChN4fa/B0hEBdAb6npDlFL8Bwx4dfRLRqAo=
 github.com/gkampitakis/go-diff v1.3.2 h1:Qyn0J9XJSDTgnsgHRdz9Zp24RaJeKMUHg2+PDZZdC4M=
 github.com/gkampitakis/go-diff v1.3.2/go.mod h1:LLgOrpqleQe26cte8s36HTWcTmMEur6OPYerdAAS9tk=
 github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=
 github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk=
 github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
+github.com/maruel/natural v1.1.0 h1:2z1NgP/Vae+gYrtC0VuvrTJ6U35OuyUqDdfluLqMWuQ=
+github.com/maruel/natural v1.1.0/go.mod h1:eFVhYCcUOfZFxXoDZam8Ktya72wa79fNC3lc/leA0DQ=
 github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA=
 github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs=
 github.com/rogpeppe/go-internal v1.11.0 h1:cWPaGQEPrBb5/AsnsZesgZZ9yb1OQ+GOISoDNXVBh4M=
 github.com/rogpeppe/go-internal v1.11.0/go.mod h1:ddIwULY96R17DhadqLgMfk9H9tvdUzkipdSkR5nkCZA=
 github.com/tidwall/gjson v1.14.2/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
-github.com/tidwall/gjson v1.16.0 h1:SyXa+dsSPpUlcwEDuKuEBJEz5vzTvOea+9rjyYodQFg=
-github.com/tidwall/gjson v1.16.0/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
+github.com/tidwall/gjson v1.17.0 h1:/Jocvlh98kcTfpN2+JzGQWQcqrPQwDrVEMApx/M5ZwM=
+github.com/tidwall/gjson v1.17.0/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
 github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA=
 github.com/tidwall/match v1.1.1/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JTxsfmM=
 github.com/tidwall/pretty v1.2.0/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU=

diff --git a/images/diff.png b/images/diff.png
diff --git a/images/new_snapshot.png b/images/new_snapshot.png
diff --git a/snaps/clean.go b/snaps/clean.go
@@ -7,18 +7,18 @@ import (
 	"io"
 	"os"
 	"path/filepath"
-	"regexp"
+	"slices"
 	"strings"
 	"sync"
 	"testing"
 
 	"github.com/gkampitakis/go-snaps/internal/colors"
+	"github.com/maruel/natural"
 )
 
 // Matches [ Test... - number ] testIDs
 var (
-	testIDRegexp = regexp.MustCompile(`(?m)^\[(Test.* - \d+)\]$`)
-	testEvents   = newTestEvents()
+	testEvents = newTestEvents()
 )
 
 const (
@@ -46,6 +46,11 @@ func newTestEvents() *events {
 	}
 }
 
+type CleanOpts struct {
+	// If set to true, `go-snaps` will sort the snapshots
+	Sort bool
+}
+
 // Clean runs checks for identifying obsolete snapshots and prints a Test Summary.
 //
 // Must be called in a TestMain
@@ -58,13 +63,34 @@ func newTestEvents() *events {
 //
 //	 os.Exit(v)
 //	}
-func Clean(t *testing.M) {
+//
+// Clean also supports options for sorting the snapshots
+//
+//	func TestMain(t *testing.M) {
+//	 v := t.Run()
+//
+//	 // After all tests have run `go-snaps` will sort snapshots
+//	 snaps.Clean(t, snaps.CleanOpts{Sort: true})
+//
+//	 os.Exit(v)
+//	}
+func Clean(t *testing.M, opts ...CleanOpts) {
+	var opt CleanOpts
+	if len(opts) != 0 {
+		opt = opts[0]
+	}
 	// This is just for making sure Clean is called from TestMain
 	_ = t
 	runOnly := flag.Lookup("test.run").Value.String()
 
-	obsoleteFiles, usedFiles := examineFiles(testsRegistry.values, runOnly, shouldClean)
-	obsoleteTests, err := examineSnaps(testsRegistry.values, usedFiles, runOnly, shouldClean)
+	obsoleteFiles, usedFiles := examineFiles(testsRegistry.values, runOnly, shouldClean && !isCI)
+	obsoleteTests, err := examineSnaps(
+		testsRegistry.values,
+		usedFiles,
+		runOnly,
+		shouldClean && !isCI,
+		opt.Sort && !isCI,
+	)
 	if err != nil {
 		fmt.Println(err)
 		return
@@ -75,11 +101,47 @@ func Clean(t *testing.M) {
 		obsoleteTests,
 		len(skippedTests.values),
 		testEvents.items,
-		shouldClean); s != "" {
+		shouldClean && !isCI,
+	); s != "" {
 		fmt.Print(s)
 	}
 }
 
+// getTestID will return the testID if the line is in the form of [Test... - number]
+func getTestID(b []byte) (string, bool) {
+	if len(b) == 0 {
+		return "", false
+	}
+
+	// needs to start with [Test and end with ]
+	if !bytes.Equal(b[0:5], []byte("[Test")) || b[len(b)-1] != ']' {
+		return "", false
+	}
+
+	// needs to contain ' - '
+	separator := bytes.Index(b, []byte(" - "))
+	if separator == -1 {
+		return "", false
+	}
+
+	// needs to have a number after the separator
+	if !isNumber(b[separator+3 : len(b)-1]) {
+		return "", false
+	}
+
+	return string(b[1 : len(b)-1]), true
+}
+
+func isNumber(b []byte) bool {
+	for i := 0; i < len(b); i++ {
+		if b[i] < '0' || b[i] > '9' {
+			return false
+		}
+	}
+
+	return true
+}
+
 /*
 Map containing the occurrences is checked against the filesystem.
 
@@ -138,21 +200,20 @@ func examineSnaps(
 	registry map[string]map[string]int,
 	used []string,
 	runOnly string,
-	shouldUpdate bool,
+	update,
+	sort bool,
 ) ([]string, error) {
 	obsoleteTests := []string{}
+	tests := map[string]string{}
+	data := bytes.Buffer{}
+	testIDs := []string{}
 
 	for _, snapPath := range used {
 		f, err := os.OpenFile(snapPath, os.O_RDWR, os.ModePerm)
 		if err != nil {
 			return nil, err
 		}
-		defer f.Close()
 
-		var updatedFile bytes.Buffer
-		if i, err := f.Stat(); err == nil {
-			updatedFile.Grow(int(i.Size()))
-		}
 		var hasDiffs bool
 
 		registeredTests := occurrences(registry[snapPath])
@@ -161,47 +222,75 @@ func examineSnaps(
 		for s.Scan() {
 			b := s.Bytes()
 			// Check if line is a test id
-			match := testIDRegexp.FindSubmatch(b)
-			if len(match) <= 1 {
+			testID, match := getTestID(b)
+			if !match {
 				continue
 			}
-			testID := string(match[1])
+			testIDs = append(testIDs, testID)
 
 			if !registeredTests.Has(testID) && !testSkipped(testID, runOnly) {
 				obsoleteTests = append(obsoleteTests, testID)
 				hasDiffs = true
 
 				removeSnapshot(s)
-
 				continue
 			}
 
-			updatedFile.WriteByte('\n')
-			updatedFile.Write(b)
-			updatedFile.WriteByte('\n')
-
 			for s.Scan() {
 				line := s.Bytes()
-				updatedFile.Write(line)
-				updatedFile.WriteByte('\n')
 
 				if bytes.Equal(line, endSequenceByteSlice) {
+					tests[testID] = data.String()
+
+					data.Reset()
 					break
 				}
+
+				data.Write(line)
+				data.WriteByte('\n')
 			}
 		}
 
 		if err := s.Err(); err != nil {
 			return nil, err
 		}
 
-		if !hasDiffs || !shouldUpdate {
+		shouldSort := sort && !slices.IsSortedFunc(testIDs, naturalSort)
+		shouldUpdate := update && hasDiffs
+
+		// if we don't have to "write" anything on the snap we skip
+		if !shouldUpdate && !shouldSort {
+			f.Close()
+
+			clear(tests)
+			testIDs = testIDs[:0]
+			data.Reset()
+
 			continue
 		}
 
-		if err = overwriteFile(f, updatedFile.Bytes()); err != nil {
-			fmt.Println(err)
+		if shouldSort {
+			// sort testIDs
+			slices.SortFunc(testIDs, naturalSort)
+		}
+
+		if err := overwriteFile(f, nil); err != nil {
+			return nil, err
+		}
+
+		for _, id := range testIDs {
+			test, ok := tests[id]
+			if !ok {
+				continue
+			}
+
+			fmt.Fprintf(f, "\n[%s]\n%s%s\n", id, test, endSequence)
 		}
+		f.Close()
+
+		clear(tests)
+		testIDs = testIDs[:0]
+		data.Reset()
 	}
 
 	return obsoleteTests, nil
@@ -329,3 +418,14 @@ func occurrences(tests map[string]int) set {
 
 	return result
 }
+
+// naturalSort is a function that can be used to sort strings in natural order
+func naturalSort(a, b string) int {
+	if a == b {
+		return 0
+	}
+	if natural.Less(a, b) {
+		return -1
+	}
+	return 1
+}