fix: extending test coverage

snaps/clean.go

@@ -33,8 +33,12 @@ func Clean() {
 	`Matchsnapshot`). If not skipped and not registed means it's an obsolete snap file
 	and we mark it as one.
 */
-func examineFiles(registry map[string]map[string]int, runOnly string, shouldUpdate bool) (obsolete []string, used []string) {
-	uniqueDirs := map[string]struct{}{}
+func examineFiles(
+	registry map[string]map[string]int,
+	runOnly string,
+	shouldUpdate bool,
+) (obsolete []string, used []string) {
+	uniqueDirs := Set{}
 
 	for snapPaths := range registry {
 		uniqueDirs[filepath.Dir(snapPaths)] = struct{}{}
@@ -75,7 +79,12 @@ func examineFiles(registry map[string]map[string]int, runOnly string, shouldUpda
 	return obsolete, used
 }
 
-func examineSnaps(registry map[string]map[string]int, used []string, runOnly string, shouldUpdate bool) ([]string, error) {
+func examineSnaps(
+	registry map[string]map[string]int,
+	used []string,
+	runOnly string,
+	shouldUpdate bool,
+) ([]string, error) {
 	obsoleteTests := []string{}
 
 	for _, snapPath := range used {

snaps/clean_test.go

@@ -121,8 +121,14 @@ func TestParseFiles(t *testing.T) {
 		tests, dir1, dir2 := setupTempParseFiles(t)
 		obsolete, used := examineFiles(tests, "", false)
 
-		obsoleteExpected := []string{filepath.FromSlash(dir1 + "/obsolete1.snap"), filepath.FromSlash(dir2 + "/obsolete2.snap")}
-		usedExpected := []string{filepath.FromSlash(dir1 + "/test1.snap"), filepath.FromSlash(dir2 + "/test2.snap")}
+		obsoleteExpected := []string{
+			filepath.FromSlash(dir1 + "/obsolete1.snap"),
+			filepath.FromSlash(dir2 + "/obsolete2.snap"),
+		}
+		usedExpected := []string{
+			filepath.FromSlash(dir1 + "/test1.snap"),
+			filepath.FromSlash(dir2 + "/test2.snap"),
+		}
 
 		// Parse files uses maps so order of strings cannot be guaranteed
 		sort.Strings(obsoleteExpected)
@@ -139,11 +145,17 @@ func TestParseFiles(t *testing.T) {
 		tests, dir1, dir2 := setupTempParseFiles(t)
 		examineFiles(tests, "", shouldUpdate)
 
-		if _, err := os.Stat(filepath.FromSlash(dir1 + "/obsolete1.snap")); !errors.Is(err, os.ErrNotExist) {
+		if _, err := os.Stat(filepath.FromSlash(dir1 + "/obsolete1.snap")); !errors.Is(
+			err,
+			os.ErrNotExist,
+		) {
 			t.Error("obsolete obsolete1.snap not removed")
 		}
 
-		if _, err := os.Stat(filepath.FromSlash(dir2 + "/obsolete2.snap")); !errors.Is(err, os.ErrNotExist) {
+		if _, err := os.Stat(filepath.FromSlash(dir2 + "/obsolete2.snap")); !errors.Is(
+			err,
+			os.ErrNotExist,
+		) {
 			t.Error("obsolete obsolete2.snap not removed")
 		}
 	})
@@ -171,7 +183,10 @@ func TestOccurrences(t *testing.T) {
 func TestParseSnaps(t *testing.T) {
 	t.Run("should report no obsolete tests", func(t *testing.T) {
 		tests, dir1, dir2 := setupTempParseFiles(t)
-		used := []string{filepath.FromSlash(dir1 + "/test1.snap"), filepath.FromSlash(dir2 + "/test2.snap")}
+		used := []string{
+			filepath.FromSlash(dir1 + "/test1.snap"),
+			filepath.FromSlash(dir2 + "/test2.snap"),
+		}
 		shouldUpdate := false
 
 		obsolete, err := examineSnaps(tests, used, "", shouldUpdate)
@@ -182,7 +197,10 @@ func TestParseSnaps(t *testing.T) {
 
 	t.Run("should report two obsolete tests and not change content", func(t *testing.T) {
 		tests, dir1, dir2 := setupTempParseFiles(t)
-		used := []string{filepath.FromSlash(dir1 + "/test1.snap"), filepath.FromSlash(dir2 + "/test2.snap")}
+		used := []string{
+			filepath.FromSlash(dir1 + "/test1.snap"),
+			filepath.FromSlash(dir2 + "/test2.snap"),
+		}
 		shouldUpdate := false
 
 		// Reducing test occurrence to 1 meaning the second test was removed ( testid - 2 )
@@ -204,7 +222,10 @@ func TestParseSnaps(t *testing.T) {
 
 	t.Run("should update the obsolete snap files", func(t *testing.T) {
 		tests, dir1, dir2 := setupTempParseFiles(t)
-		used := []string{filepath.FromSlash(dir1 + "/test1.snap"), filepath.FromSlash(dir2 + "/test2.snap")}
+		used := []string{
+			filepath.FromSlash(dir1 + "/test1.snap"),
+			filepath.FromSlash(dir2 + "/test2.snap"),
+		}
 		shouldUpdate := true
 
 		delete(tests[used[0]], "TestDir1_3/TestSimple")

snaps/skip.go

@@ -35,7 +35,8 @@ func SkipNow(t TestingT) {
 }
 
 /*
-	This checks if the parent test is skipped
+	This checks if the parent test is skipped,
+	or provided a 'runOnly' the testID is part of it
 	e.g
 	func TestParallel (t *testing.T) {
 		snaps.Skip()
@@ -63,7 +64,7 @@ func isFileSkipped(dir, filename, runOnly string) bool {
 		return false
 	}
 
-	testFilePath := path.Join(dir, "..", strings.TrimSuffix(filename, ".snap")+".go")
+	testFilePath := path.Join(dir, "..", strings.TrimSuffix(filename, snapsExt)+".go")
 	isSkipped := true
 
 	fset := token.NewFileSet()

snaps/skip_test.go

@@ -1,75 +1,62 @@
 package snaps
 
 import (
+	"os"
 	"sync"
 	"testing"
 )
 
 type MockTestingT struct {
-	calls map[string]interface{}
-	// skip spies calls [this is for data race detection in last test]
-	skip bool
+	mockHelper  func()
+	mockName    func() string
+	mockSkip    func(args ...interface{})
+	mockSkipf   func(format string, args ...interface{})
+	mockSkipNow func()
+	mockError   func(args ...interface{})
 }
 
-func (m *MockTestingT) Helper() {
-	if m.skip {
-		return
-	}
-	if v, exists := m.calls["Helper"]; exists {
-		m.calls["Helper"] = v.(int) + 1
-		return
-	}
-
-	m.calls["Helper"] = +1
+func (m MockTestingT) Error(args ...interface{}) {
+	m.mockError(args...)
 }
 
-func (m *MockTestingT) Skip(args ...interface{}) {
-	if m.skip {
-		return
-	}
-	m.calls["Skip"] = args
+func (m MockTestingT) Helper() {
+	m.mockHelper()
 }
 
-func (m *MockTestingT) Skipf(format string, args ...interface{}) {
-	if m.skip {
-		return
-	}
-	if _, exists := m.calls["Skipf"]; !exists {
-		m.calls["Skipf"] = []interface{}{}
-	}
-	m.calls["Skipf"] = append(m.calls["Skipf"].([]interface{}), format)
-	m.calls["Skipf"] = append(m.calls["Skipf"].([]interface{}), args...)
+func (m MockTestingT) Skip(args ...interface{}) {
+	m.mockSkip(args...)
 }
 
-func (m *MockTestingT) SkipNow() {
-	if m.skip {
-		return
-	}
-	if v, exists := m.calls["SkipNow"]; exists {
-		m.calls["SkipNow"] = v.(int) + 1
-		return
-	}
+func (m MockTestingT) Skipf(format string, args ...interface{}) {
+	m.mockSkipf(format, args...)
+}
 
-	m.calls["SkipNow"] = +1
+func (m MockTestingT) SkipNow() {
+	m.mockSkipNow()
 }
 
 func (m MockTestingT) Name() string {
-	return "mock-test"
+	return m.mockName()
 }
 
 func TestSkip(t *testing.T) {
 	t.Run("should call Skip", func(t *testing.T) {
 		t.Cleanup(func() {
 			skippedTests = newSyncSlice()
 		})
+		skipArgs := []interface{}{1, 2, 3, 4, 5}
 
 		mockT := MockTestingT{
-			calls: make(map[string]interface{}, 5),
+			mockSkip: func(args ...interface{}) {
+				Equal(t, skipArgs, args)
+			},
+			mockHelper: func() {},
+			mockName: func() string {
+				return "mock-test"
+			},
 		}
-		Skip(&mockT, 1, 2, 3, 4, 5)
+		Skip(mockT, 1, 2, 3, 4, 5)
 
-		Equal(t, 1, mockT.calls["Helper"])
-		Equal(t, []interface{}{1, 2, 3, 4, 5}, mockT.calls["Skip"])
 		Equal(t, []string{"mock-test"}, skippedTests.values)
 	})
 
@@ -79,12 +66,17 @@ func TestSkip(t *testing.T) {
 		})
 
 		mockT := MockTestingT{
-			calls: make(map[string]interface{}, 5),
+			mockSkipf: func(format string, args ...interface{}) {
+				Equal(t, "mock", format)
+				Equal(t, []interface{}{1, 2, 3, 4, 5}, args)
+			},
+			mockHelper: func() {},
+			mockName: func() string {
+				return "mock-test"
+			},
 		}
-		Skipf(&mockT, "mock", 1, 2, 3, 4, 5)
+		Skipf(mockT, "mock", 1, 2, 3, 4, 5)
 
-		Equal(t, 1, mockT.calls["Helper"])
-		Equal(t, []interface{}{"mock", 1, 2, 3, 4, 5}, mockT.calls["Skipf"])
 		Equal(t, []string{"mock-test"}, skippedTests.values)
 	})
 
@@ -94,12 +86,14 @@ func TestSkip(t *testing.T) {
 		})
 
 		mockT := MockTestingT{
-			calls: make(map[string]interface{}, 5),
+			mockSkipNow: func() {},
+			mockHelper:  func() {},
+			mockName: func() string {
+				return "mock-test"
+			},
 		}
-		SkipNow(&mockT)
+		SkipNow(mockT)
 
-		Equal(t, 1, mockT.calls["Helper"])
-		Equal(t, 1, mockT.calls["SkipNow"])
 		Equal(t, []string{"mock-test"}, skippedTests.values)
 	})
 
@@ -108,24 +102,87 @@ func TestSkip(t *testing.T) {
 			skippedTests = newSyncSlice()
 		})
 
-		wg := sync.WaitGroup{}
-
 		mockT := MockTestingT{
-			calls: make(map[string]interface{}, 5),
-			skip:  true,
+			mockSkipNow: func() {},
+			mockHelper:  func() {},
+			mockName: func() string {
+				return "mock-test"
+			},
 		}
 
+		wg := sync.WaitGroup{}
+
 		for i := 0; i < 1000; i++ {
 			wg.Add(1)
 
 			go func() {
 				defer wg.Done()
-				SkipNow(&mockT)
+				SkipNow(mockT)
 			}()
 		}
 
 		wg.Wait()
 
 		Equal(t, 1000, len(skippedTests.values))
 	})
+
+	t.Run("testSkipped", func(t *testing.T) {
+		t.Run("should return true if testID is not part of the 'runOnly'", func(t *testing.T) {
+			runOnly := "TestMock"
+			testID := "TestSkip/should_call_Skip - 1"
+
+			received := testSkipped(testID, runOnly)
+			Equal(t, true, received)
+		})
+
+		t.Run("should return false if testID is part of 'runOnly'", func(t *testing.T) {
+			runOnly := "TestMock"
+			testID := "TestMock/Test/should_be_not_skipped - 2"
+
+			received := testSkipped(testID, runOnly)
+			Equal(t, false, received)
+		})
+
+		t.Run(
+			"should check if the parent is skipped and mark child tests as skipped",
+			func(t *testing.T) {
+				t.Cleanup(func() {
+					skippedTests = newSyncSlice()
+				})
+
+				runOnly := ""
+				mockT := MockTestingT{
+					mockSkipNow: func() {},
+					mockHelper:  func() {},
+					mockName: func() string {
+						return "TestMock/Skip"
+					},
+				}
+				// This is for populating skippedTests.values and following the normal flow
+				SkipNow(mockT)
+
+				Equal(t, true, testSkipped("TestMock/Skip", runOnly))
+				Equal(t, true, testSkipped("TestMock/Skip/child_should_also_be_skipped", runOnly))
+				Equal(t, false, testSkipped("TestAnotherTest", runOnly))
+			},
+		)
+	})
+
+	t.Run("isFileSkipped", func(t *testing.T) {
+		t.Run("should return 'false'", func(t *testing.T) {
+			Equal(t, false, isFileSkipped("", "", ""))
+		})
+
+		t.Run("should return 'true' if test is not included in the test file", func(t *testing.T) {
+			dir, _ := os.Getwd()
+
+			Equal(t, true, isFileSkipped(dir+"/__snapshots__", "skip_test.snap", "TestNonExistent"))
+		})
+
+		t.Run("should return 'false' if test is included in the test file", func(t *testing.T) {
+			dir, _ := os.Getwd()
+
+			Equal(t, false, isFileSkipped(dir+"/__snapshots__", "skip_test.snap", "TestSkip"))
+		})
+	})
 }

snaps/snaps.go

@@ -7,8 +7,6 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
-
-	"github.com/gkampitakis/ciinfo"
 )
 
 func MatchSnapshot(t *testing.T, o ...interface{}) {
@@ -18,7 +16,7 @@ func MatchSnapshot(t *testing.T, o ...interface{}) {
 }
 
 // Using pointer here so we can remove the interface{} when printing
-func matchSnapshot(t *testing.T, o *[]interface{}) {
+func matchSnapshot(t TestingT, o *[]interface{}) {
 	t.Helper()
 
 	if len(*o) == 0 {
@@ -30,7 +28,12 @@ func matchSnapshot(t *testing.T, o *[]interface{}) {
 	snapshot := takeSnapshot(o)
 	prevSnapshot, err := getPrevSnapshot(testID, snapPath)
 
-	if errors.Is(err, errSnapNotFound) && !ciinfo.IsCI {
+	if errors.Is(err, errSnapNotFound) {
+		if isCI {
+			t.Error(err)
+			return
+		}
+
 		err := addNewSnapshot(testID, snapshot, dir, snapPath)
 		if err != nil {
 			t.Error(err)
@@ -121,7 +124,7 @@ func snapDirAndName() (dir, name string) {
 	base := filepath.Base(callerPath)
 
 	dir = filepath.Join(filepath.Dir(callerPath), snapsDir)
-	name = filepath.Join(dir, strings.TrimSuffix(base, filepath.Ext(base))+"."+snapsExt)
+	name = filepath.Join(dir, strings.TrimSuffix(base, filepath.Ext(base))+snapsExt)
 
 	return
 }