Change implementation to use interface instead of function + PR feedback

Author: renzoarreaza

suite/interfaces.go

@@ -64,3 +64,9 @@ type SetupSubTest interface {
 type TearDownSubTest interface {
 	TearDownSubTest()
 }
+
+// SkipTest has a SkipTest method, which will run for every test during the
+// collection phase.
+type SkipTest interface {
+	SkipTest(testSuiteName, testName string) bool
+}

suite/suite.go

@@ -119,19 +119,11 @@ func (suite *Suite) Run(name string, subtest func()) bool {
 // Run takes a testing suite and runs all of the tests attached
 // to it.
 func Run(t *testing.T, suite TestingSuite) {
-	RunWithSkip(t, suite, func(_, _ string) bool { return false })
-}
-
-// RunWithSkip takes a testing suite and a skip function allowing
-// for extra filtering on tests to be run
-func RunWithSkip(t *testing.T, suite TestingSuite, skip func(string, string) bool) {
 	defer recoverAndFailOnPanic(t)
 
 	suite.SetT(t)
 	suite.SetS(suite)
 
-	var suiteSetupDone bool
-
 	var stats *SuiteInformation
 	if _, ok := suite.(WithStats); ok {
 		stats = newSuiteInformation()
@@ -154,8 +146,10 @@ func RunWithSkip(t *testing.T, suite TestingSuite, skip func(string, string) boo
 			continue
 		}
 
-		if skip(suiteName, method.Name) {
-			continue
+		if skipTest, ok := suite.(SkipTest); ok {
+			if skipTest.SkipTest(suiteName, method.Name) {
+				continue
+			}
 		}
 
 		test := testing.InternalTest{
@@ -203,30 +197,28 @@ func RunWithSkip(t *testing.T, suite TestingSuite, skip func(string, string) boo
 		tests = append(tests, test)
 	}
 
-	if len(tests) > 0 {
-		if stats != nil {
-			stats.Start = time.Now()
-		}
+	if len(tests) == 0 {
+		return
+	}
 
-		if setupAllSuite, ok := suite.(SetupAllSuite); ok {
-			setupAllSuite.SetupSuite()
-		}
+	if stats != nil {
+		stats.Start = time.Now()
+	}
 
-		suiteSetupDone = true
+	if setupAllSuite, ok := suite.(SetupAllSuite); ok {
+		setupAllSuite.SetupSuite()
 	}
 
-	if suiteSetupDone {
-		defer func() {
-			if tearDownAllSuite, ok := suite.(TearDownAllSuite); ok {
-				tearDownAllSuite.TearDownSuite()
-			}
+	defer func() {
+		if tearDownAllSuite, ok := suite.(TearDownAllSuite); ok {
+			tearDownAllSuite.TearDownSuite()
+		}
 
-			if suiteWithStats, measureStats := suite.(WithStats); measureStats {
-				stats.End = time.Now()
-				suiteWithStats.HandleStats(suiteName, stats)
-			}
-		}()
-	}
+		if suiteWithStats, measureStats := suite.(WithStats); measureStats {
+			stats.End = time.Now()
+			suiteWithStats.HandleStats(suiteName, stats)
+		}
+	}()
 
 	runTests(t, tests)
 }

suite/suite_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"errors"
 	"flag"
+	"fmt"
 	"io"
 	"math/rand"
 	"os"
@@ -747,17 +748,83 @@ func TestUnInitializedSuites(t *testing.T) {
 	})
 }
 
-var register = map[string]bool{"SuiteTesterTestTwo": true}
+// Test/Example of SkipTest
 
-func skip(suiteName, testName string) bool {
-	return register[suiteName+testName]
+type buildVersion struct {
+	major int
+	minor int
+}
+
+func (v buildVersion) after(other buildVersion) bool {
+	if v.major > other.major {
+		return true
+	}
+	if v.major < other.major {
+		return false
+	}
+	if v.minor > other.minor {
+		return true
+	}
+	return false
+}
+
+type testRegistration struct {
+	release  buildVersion
+	features []string
+}
+
+var testRegistrations = map[string]testRegistration{
+	"SuiteWithSkipTestOne": {buildVersion{1, 2}, []string{"myOldFeature"}},
+	"SuiteWithSkipTestTwo": {buildVersion{2, 0}, []string{"myNewFeature"}},
+}
+
+func contains(s []string, v string) bool {
+	for i := range s {
+		if v == s[i] {
+			return true
+		}
+	}
+	return false
+}
+
+// reusing the Suite struct defined above
+type SuiteWithSkip struct {
+	SuiteTester
+}
+
+// SkipTest Implements the SkipTest interface
+func (s *SuiteWithSkip) SkipTest(testSuiteName string, testName string) bool {
+	testRegister, ok := testRegistrations[testSuiteName+testName]
+	if !ok {
+		return false
+	}
+
+	// runParameters.
+	// These could for example be defined in a file, or as cli arguments.
+	// In this example we define them here
+	currentVersion := buildVersion{1, 2}
+	enabledFeatures := []string{"myOldFeature", "myNewFeature"}
+
+	if testRegister.release.after(currentVersion) {
+		fmt.Printf("Skipping %s, due to release", testName)
+		return true
+	}
+
+	for _, registeredFeature := range testRegister.features {
+		if !contains(enabledFeatures, registeredFeature) {
+			fmt.Printf("Skipping %s, due to feature", testName)
+			return true
+		}
+	}
+
+	return false
 }
 
 // TestRunSuiteWithSkip will be run by the 'go test' command, so within it, we
 // can run our suite using the Run(*testing.T, TestingSuite) function.
 func TestRunSuiteWithSkip(t *testing.T) {
-	suiteTester := new(SuiteTester)
-	RunWithSkip(t, suiteTester, skip)
+	suiteTester := new(SuiteWithSkip)
+	Run(t, suiteTester)
 
 	// Normally, the test would end here.  The following are simply
 	// some assertions to ensure that the Run function is working as
@@ -790,11 +857,11 @@ func TestRunSuiteWithSkip(t *testing.T) {
 	assert.Contains(t, suiteTester.TearDownSubTestNames, "TestRunSuiteWithSkip/TestSubtest/second")
 
 	for _, suiteName := range suiteTester.SuiteNameAfter {
-		assert.Equal(t, "SuiteTester", suiteName)
+		assert.Equal(t, "SuiteWithSkip", suiteName)
 	}
 
 	for _, suiteName := range suiteTester.SuiteNameBefore {
-		assert.Equal(t, "SuiteTester", suiteName)
+		assert.Equal(t, "SuiteWithSkip", suiteName)
 	}
 
 	for _, when := range suiteTester.TimeAfter {
@@ -834,11 +901,22 @@ func TestRunSuiteWithSkip(t *testing.T) {
 
 }
 
+// SuiteWithSkipAll reuses the Suite struct defined above and implements the SkipTest interface skipping all tests
+type SuiteWithSkipAll struct {
+	SuiteTester
+}
+
+// SkipTest Implements the SkipTest interface all tests and Setup/TeardownSuite will be skipped
+func (s *SuiteWithSkipAll) SkipTest(testSuiteName string, testName string) bool {
+	return true
+}
+
 // TestRunSuiteWithSkip will be run by the 'go test' command, so within it, we
 // can run our suite using the Run(*testing.T, TestingSuite) function.
+
 func TestRunSuiteWithSkipAll(t *testing.T) {
-	suiteTester := new(SuiteTester)
-	RunWithSkip(t, suiteTester, func(_, _ string) bool { return true })
+	suiteTester := new(SuiteWithSkipAll)
+	Run(t, suiteTester)
 
 	// Normally, the test would end here.  The following are simply
 	// some assertions to ensure that the Run function is working as
@@ -871,11 +949,11 @@ func TestRunSuiteWithSkipAll(t *testing.T) {
 	assert.NotContains(t, suiteTester.TearDownSubTestNames, "TestRunSuiteWithSkipAll/TestSubtest/second")
 
 	for _, suiteName := range suiteTester.SuiteNameAfter {
-		assert.Equal(t, "SuiteTester", suiteName)
+		assert.Equal(t, "SuiteWithSkipAll", suiteName)
 	}
 
 	for _, suiteName := range suiteTester.SuiteNameBefore {
-		assert.Equal(t, "SuiteTester", suiteName)
+		assert.Equal(t, "SuiteWithSkipAll", suiteName)
 	}
 
 	for _, when := range suiteTester.TimeAfter {