feat: make messages more consistent (#16)

gkampitakis · Jan 2, 2022 · 1809655 · 1809655
1 parent a08a28a
commit 1809655
Show file tree

Hide file tree

Showing 12 changed files with 355 additions and 56 deletions.
diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml
@@ -19,6 +19,7 @@ jobs:
         uses: golangci/golangci-lint-action@v2
         with:
           version: "latest"
+          args: -c ./golangci.yml
   test:
     name: Run tests
     runs-on: ${{ matrix.os }}
@@ -37,4 +38,4 @@ jobs:
         with:
           go-version: ${{matrix.go}}
       - name: Run Tests
-        run: go test -cover -race ./...
+        run: go test -cover -race -v ./...
diff --git a/README.md b/README.md
@@ -105,6 +105,11 @@ map[string]interface {}{
 
 ### Clean obsolete snapshots
 
+<p align="center">
+<img src="./images/summary-obsolete.png" alt="Summary Obsolete" width="400"/>
+<img src="./images/summary-removed.png" alt="Summary Removed" width="500"/>
+</p>
+
 `go-snaps` can identify obsolete snapshots.
 
 In order to enable this functionality you need to use the `TestMain(t*testing.M)` 

diff --git a/golangci.yml b/golangci.yml
@@ -9,6 +9,9 @@ linters:
     - goimports
     - unparam
 
+  disable:
+    - errcheck
+
 linters-settings:
   lll:
-    line-length: 180
+    line-length: 130
diff --git a/images/summary-obsolete.png b/images/summary-obsolete.png
diff --git a/images/summary-removed.png b/images/summary-removed.png
diff --git a/snaps/clean.go b/snaps/clean.go
@@ -27,13 +27,13 @@ func Clean() {
 	}
 	runOnly := parseRunFlag(os.Args)
 
-	obsoleteSnaps, usedSnaps := examineFiles(testsRegistry.values, runOnly, shouldUpdate)
-	obsoleteTests, err := examineSnaps(testsRegistry.values, usedSnaps, runOnly, shouldUpdate)
+	obsoleteFiles, usedFiles := examineFiles(testsRegistry.values, runOnly, shouldUpdate)
+	obsoleteTests, err := examineSnaps(testsRegistry.values, usedFiles, runOnly, shouldUpdate)
 	if err != nil {
 		fmt.Println(err)
 	}
 
-	summary(obsoleteSnaps, obsoleteTests)
+	summary(fmt.Printf, obsoleteFiles, obsoleteTests, shouldUpdate)
 }
 
 /*
@@ -154,31 +154,71 @@ func examineSnaps(
 	return obsoleteTests, nil
 }
 
-func summary(obsoleteSnaps []string, obsoleteTests []string) {
-	if len(obsoleteSnaps) == 0 && len(obsoleteTests) == 0 {
+func summary(print printerF, obsoleteFiles []string, obsoleteTests []string, shouldUpdate bool) {
+	if len(obsoleteFiles) == 0 && len(obsoleteTests) == 0 {
 		return
 	}
 
-	fmt.Printf(" %s	\n", greenBG("Snapshot Summary"))
+	print("\n%s\n\n", greenBG("Snapshot Summary"))
 
-	if len(obsoleteSnaps) > 0 {
-		if len(obsoleteSnaps) > 2 {
-			fmt.Println(yellowText(fmt.Sprintf("%d obsolete files detected", len(obsoleteSnaps))))
-		} else {
-			fmt.Println(yellowText(fmt.Sprintf("%d obsolete file detected", len(obsoleteSnaps))))
-		}
+	if len(obsoleteFiles) > 0 {
+		print(summaryMsg(
+			len(obsoleteFiles),
+			stringTernary("files", "file", len(obsoleteFiles) > 1),
+			shouldUpdate),
+		)
 
-		for _, file := range obsoleteSnaps {
-			fmt.Println(dimText("	" + file))
+		for _, file := range obsoleteFiles {
+			print(dimText(fmt.Sprintf("  %s%s\n", bulletPoint, file)))
 		}
+
+		print("\n")
 	}
 
 	if len(obsoleteTests) > 0 {
-		fmt.Println(yellowText(fmt.Sprintf("%d obsolete tests detected", len(obsoleteTests))))
+		print(summaryMsg(
+			len(obsoleteTests),
+			stringTernary("tests", "test", len(obsoleteTests) > 1),
+			shouldUpdate),
+		)
+
 		for _, test := range obsoleteTests {
-			fmt.Println(dimText("	" + test))
+			print(dimText(fmt.Sprintf("  %s%s\n", bulletPoint, test)))
 		}
+
+		print("\n")
 	}
+
+	if !shouldUpdate {
+		print(dimText("You can remove obsolete files and tests by running 'UPDATE_SNAPS=true go test ./...'\n"))
+	}
+}
+
+func summaryMsg(files int, subject string, updated bool) string {
+	action := stringTernary("removed", "obsolete", updated)
+	color := colorTernary(greenText, yellowText, updated)
+
+	return color(fmt.Sprintf("%s%d snapshot %s %s.\n", arrowPoint, files, subject, action))
+}
+
+func stringTernary(trueBranch string, falseBranch string, assertion bool) string {
+	if !assertion {
+		return falseBranch
+	}
+
+	return trueBranch
+}
+
+func colorTernary(
+	colorFuncTrue func(string) string,
+	colorFuncFalse func(string) string,
+	assertion bool,
+) func(string) string {
+	if !assertion {
+		return colorFuncFalse
+	}
+
+	return colorFuncTrue
 }
 
 /*

diff --git a/snaps/clean_test.go b/snaps/clean_test.go
@@ -2,6 +2,7 @@ package snaps
 
 import (
 	"errors"
+	"fmt"
 	"os"
 	"path/filepath"
 	"sort"
@@ -161,25 +162,6 @@ func TestExamineFiles(t *testing.T) {
 	})
 }
 
-func TestOccurrences(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":   {},
-	}
-
-	Equal(t, expected, occurrences(tests))
-}
-
 func TestExamineSnaps(t *testing.T) {
 	t.Run("should report no obsolete tests", func(t *testing.T) {
 		tests, dir1, dir2 := setupTempExamineFiles(t)
@@ -270,6 +252,25 @@ string hello world 2 2 1
 	})
 }
 
+func TestOccurrences(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":   {},
+	}
+
+	Equal(t, expected, occurrences(tests))
+}
+
 func TestParseRunFlag(t *testing.T) {
 	t.Run("should return empty string", func(t *testing.T) {
 		runOly := parseRunFlag([]string{"-test.flag=ignore"})
@@ -283,3 +284,178 @@ func TestParseRunFlag(t *testing.T) {
 		Equal(t, "MyTest", runOly)
 	})
 }
+
+func TestSummary(t *testing.T) {
+	t.Run("should not print anything", func(t *testing.T) {
+		mockPrinter := func(format string, args ...interface{}) (int, error) {
+			NotCalled(t)
+			return 0, nil
+		}
+
+		summary(mockPrinter, nil, nil, false)
+	})
+
+	t.Run("should print obsolete files", func(t *testing.T) {
+		expectedCalls := []func(format string, args ...interface{}){
+			func(format string, args ...interface{}) {
+				expectedFormat := "\n%s\n\n"
+				Equal(t, expectedFormat, format)
+
+				expectedArg := greenBG("Snapshot Summary")
+				Equal(t, expectedArg, args[0])
+			},
+			func(format string, arg ...interface{}) {
+				expected := yellowText(
+					fmt.Sprintf("%s%d snapshot files obsolete.\n", arrowPoint, 2),
+				)
+				Equal(t, expected, format)
+			},
+			func(format string, args ...interface{}) {
+				expected := dimText("  " + bulletPoint + "test0.snap\n")
+				Equal(t, expected, format)
+			},
+			func(format string, args ...interface{}) {
+				expected := dimText("  " + bulletPoint + "test1.snap\n")
+				Equal(t, expected, format)
+			},
+			func(format string, args ...interface{}) {
+				expected := "\n"
+				Equal(t, expected, format)
+			},
+			func(format string, args ...interface{}) {
+				expected := dimText("You can remove obsolete files and tests by running 'UPDATE_SNAPS=true go test ./...'\n")
+				Equal(t, expected, format)
+			},
+			func(format string, args ...interface{}) {
+				NotCalled(t)
+			},
+		}
+		mockPrinter := func(format string, args ...interface{}) (int, error) {
+			expectedCalls[0](format, args...)
+
+			expectedCalls = expectedCalls[1:]
+			return 0, nil
+		}
+
+		summary(mockPrinter, []string{"test0.snap", "test1.snap"}, nil, false)
+	})
+
+	t.Run("should print obsolete tests", func(t *testing.T) {
+		expectedCalls := []func(format string, args ...interface{}){
+			func(format string, args ...interface{}) {
+				expectedFormat := "\n%s\n\n"
+				Equal(t, expectedFormat, format)
+
+				expectedArg := greenBG("Snapshot Summary")
+				Equal(t, expectedArg, args[0])
+			},
+			func(format string, arg ...interface{}) {
+				expected := yellowText(
+					fmt.Sprintf("%s%d snapshot tests obsolete.\n", arrowPoint, 2),
+				)
+				Equal(t, expected, format)
+			},
+			func(format string, args ...interface{}) {
+				expected := dimText("  " + bulletPoint + "TestMock/should_pass - 1\n")
+				Equal(t, expected, format)
+			},
+			func(format string, args ...interface{}) {
+				expected := dimText("  " + bulletPoint + "TestMock/should_pass - 2\n")
+				Equal(t, expected, format)
+			},
+			func(format string, args ...interface{}) {
+				expected := "\n"
+				Equal(t, expected, format)
+			},
+			func(format string, args ...interface{}) {
+				expected := dimText("You can remove obsolete files and tests by running 'UPDATE_SNAPS=true go test ./...'\n")
+				Equal(t, expected, format)
+			},
+			func(format string, args ...interface{}) {
+				NotCalled(t)
+			},
+		}
+		mockPrinter := func(format string, args ...interface{}) (int, error) {
+			expectedCalls[0](format, args...)
+
+			expectedCalls = expectedCalls[1:]
+			return 0, nil
+		}
+
+		summary(mockPrinter, nil, []string{"TestMock/should_pass - 1", "TestMock/should_pass - 2"}, false)
+	})
+
+	t.Run("should print updated file", func(t *testing.T) {
+		expectedCalls := []func(format string, args ...interface{}){
+			func(format string, args ...interface{}) {
+				expectedFormat := "\n%s\n\n"
+				Equal(t, expectedFormat, format)
+
+				expectedArg := greenBG("Snapshot Summary")
+				Equal(t, expectedArg, args[0])
+			},
+			func(format string, arg ...interface{}) {
+				expected := greenText(
+					fmt.Sprintf("%s%d snapshot file removed.\n", arrowPoint, 1),
+				)
+				Equal(t, expected, format)
+			},
+			func(format string, args ...interface{}) {
+				expected := dimText("  " + bulletPoint + "test0.snap\n")
+				Equal(t, expected, format)
+			},
+			func(format string, args ...interface{}) {
+				expected := "\n"
+				Equal(t, expected, format)
+			},
+			func(format string, args ...interface{}) {
+				NotCalled(t)
+			},
+		}
+		mockPrinter := func(format string, args ...interface{}) (int, error) {
+			expectedCalls[0](format, args...)
+
+			expectedCalls = expectedCalls[1:]
+			return 0, nil
+		}
+
+		summary(mockPrinter, []string{"test0.snap"}, nil, true)
+	})
+
+	t.Run("should print updated test", func(t *testing.T) {
+		expectedCalls := []func(format string, args ...interface{}){
+			func(format string, args ...interface{}) {
+				expectedFormat := "\n%s\n\n"
+				Equal(t, expectedFormat, format)
+
+				expectedArg := greenBG("Snapshot Summary")
+				Equal(t, expectedArg, args[0])
+			},
+			func(format string, arg ...interface{}) {
+				expected := greenText(
+					fmt.Sprintf("%s%d snapshot test removed.\n", arrowPoint, 1),
+				)
+				Equal(t, expected, format)
+			},
+			func(format string, args ...interface{}) {
+				expected := dimText("  " + bulletPoint + "TestMock/should_pass - 1\n")
+				Equal(t, expected, format)
+			},
+			func(format string, args ...interface{}) {
+				expected := "\n"
+				Equal(t, expected, format)
+			},
+			func(format string, args ...interface{}) {
+				NotCalled(t)
+			},
+		}
+		mockPrinter := func(format string, args ...interface{}) (int, error) {
+			expectedCalls[0](format, args...)
+
+			expectedCalls = expectedCalls[1:]
+			return 0, nil
+		}
+
+		summary(mockPrinter, nil, []string{"TestMock/should_pass - 1"}, true)
+	})
+}
diff --git a/snaps/skip_test.go b/snaps/skip_test.go
@@ -13,6 +13,7 @@ type MockTestingT struct {
 	mockSkipf   func(format string, args ...interface{})
 	mockSkipNow func()
 	mockError   func(args ...interface{})
+	mockLog     func(args ...interface{})
 }
 
 func (m MockTestingT) Error(args ...interface{}) {
@@ -39,6 +40,10 @@ func (m MockTestingT) Name() string {
 	return m.mockName()
 }
 
+func (m MockTestingT) Log(args ...interface{}) {
+	m.mockLog(args...)
+}
+
 func TestSkip(t *testing.T) {
 	t.Run("should call Skip", func(t *testing.T) {
 		t.Cleanup(func() {