Prefer timeouts in order of -> spec, suite, default

Author: stbenjam

pkg/test/ginkgo/cmd_runsuite.go

@@ -20,6 +20,7 @@ import (
 	"github.com/onsi/ginkgo/v2"
 	"github.com/openshift-eng/openshift-tests-extension/pkg/extension"
 	configv1 "github.com/openshift/api/config/v1"
+	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/pflag"
 	"golang.org/x/mod/semver"
@@ -244,7 +245,10 @@ func (o *GinkgoRunSuiteOptions) Run(suite *TestSuite, clusterConfig *clusterdisc
 		return err
 	}
 
-	tests := externalBinaryTestsToOriginTestCases(specs)
+	tests, err := extensionTestSpecsToOriginTestCases(specs)
+	if err != nil {
+		return errors.WithMessage(err, "could not convert test specs to origin test cases")
+	}
 
 	// this ensures the tests are always run in random order to avoid
 	// any intra-tests dependencies

pkg/test/ginkgo/test_runner.go

@@ -313,7 +313,12 @@ func (c *commandContext) RunTestInNewProcess(ctx context.Context, test *testCase
 		return ret
 	}
 
-	results := test.binary.RunTests(ctx, c.timeout, testEnv, test.name)
+	timeout := c.timeout
+	if test.testTimeout > 0 {
+		timeout = test.testTimeout
+	}
+
+	results := test.binary.RunTests(ctx, timeout, testEnv, test.name)
 	if len(results) != 1 {
 		fmt.Fprintf(os.Stderr, "warning: expected 1 result from external binary; received %d", len(results))
 	}

pkg/test/ginkgo/test_suite.go

@@ -5,65 +5,37 @@ import (
 	"time"
 
 	"github.com/onsi/ginkgo/v2/types"
-	"github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests"
-	"k8s.io/apimachinery/pkg/util/errors"
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 
 	"github.com/openshift/origin/pkg/test/extensions"
 )
 
-func internalTestSpecsToOriginTestCases(suite *TestSuite, specs extensiontests.ExtensionTestSpecs) ([]*testCase, error) {
-	var tests []*testCase
-	var errs []error
-
-	specs.Walk(func(spec *extensiontests.ExtensionTestSpec) {
-		tc := &testCase{
-			name:    spec.Name,
-			rawName: spec.Name,
-		}
-		if suite != nil && suite.TestTimeout > 0 {
-			tc.testTimeout = suite.TestTimeout
-		}
-
-		tests = append(tests, tc)
-	})
-	if len(errs) > 0 {
-		return nil, errors.NewAggregate(errs)
-	}
-
-	return tests, nil
-}
-
 var re = regexp.MustCompile(`.*\[Timeout:(.[^\]]*)\]`)
 
-func externalBinaryTestsToOriginTestCases(specs extensions.ExtensionTestSpecs) []*testCase {
+func extensionTestSpecsToOriginTestCases(specs extensions.ExtensionTestSpecs) ([]*testCase, error) {
 	var tests []*testCase
 	for _, spec := range specs {
-		tests = append(tests, &testCase{
+		tc := &testCase{
 			name:    spec.Name,
 			rawName: spec.Name,
 			binary:  spec.Binary,
-		})
-	}
-	return tests
-}
-
-func newTestCaseFromGinkgoSpec(spec types.TestSpec) (*testCase, error) {
-	name := spec.Text()
-	tc := &testCase{
-		name:      name,
-		locations: spec.CodeLocations(),
-		spec:      spec,
-	}
+		}
 
-	if match := re.FindStringSubmatch(name); match != nil {
-		testTimeOut, err := time.ParseDuration(match[1])
-		if err != nil {
-			return nil, err
+		// Override timeout from suite with `[Timeout:X]` duration
+		if match := re.FindStringSubmatch(tc.name); match != nil {
+			testTimeOut, err := time.ParseDuration(match[1])
+			if err != nil {
+				return nil, errors.WithMessage(err, "failed to parse test timeout")
+			}
+			logrus.WithField("test", tc.name).Debugf("Overriding test timeout to %s", testTimeOut)
+			tc.testTimeout = testTimeOut
 		}
-		tc.testTimeout = testTimeOut
+
+		tests = append(tests, tc)
 	}
 
-	return tc, nil
+	return tests, nil
 }
 
 type testCase struct {
@@ -161,8 +133,6 @@ type TestSuite struct {
 	Extension  *extensions.Extension `json:"-"`
 }
 
-
-
 type TestMatchFunc func(name string) bool
 
 func (s *TestSuite) Filter(tests []*testCase) []*testCase {