From 529991952f4c8608acd85cf1b8536dbea0b59b05 Mon Sep 17 00:00:00 2001 From: Mattia Lavacca Date: Wed, 11 Sep 2024 17:45:11 +0200 Subject: [PATCH] Signal test stability (#3313) * feat: signal stability of tests Signed-off-by: Mattia Lavacca * chore: UDPRoute test has been marked as trial Signed-off-by: Mattia Lavacca * chore: lint Signed-off-by: Mattia Lavacca * SucceededTrailTests sorted in report Signed-off-by: Mattia Lavacca * chore: trial renamed to provisional Signed-off-by: Mattia Lavacca --------- Signed-off-by: Mattia Lavacca --- conformance/apis/v1/conformancereport.go | 4 + conformance/conformance.go | 1 + conformance/tests/udproute-simple.go | 1 + conformance/utils/flags/flags.go | 1 + conformance/utils/suite/conformance.go | 1 + conformance/utils/suite/reports.go | 20 +- conformance/utils/suite/suite.go | 37 +++- conformance/utils/suite/suite_test.go | 231 +++++++++++++++++++++++ 8 files changed, 274 insertions(+), 22 deletions(-) diff --git a/conformance/apis/v1/conformancereport.go b/conformance/apis/v1/conformancereport.go index 1e664f9acd..94e5e8e064 100644 --- a/conformance/apis/v1/conformancereport.go +++ b/conformance/apis/v1/conformancereport.go @@ -46,6 +46,10 @@ type ConformanceReport struct { // ProfileReports is a list of the individual reports for each conformance // profile that was enabled for a test run. ProfileReports []ProfileReport `json:"profiles"` + + // SucceededProvisionalTests is a list of the names of the provisional tests that + // have been successfully run. + SucceededProvisionalTests []string `json:"succeededProvisionalTests,omitempty"` } // Implementation provides metadata information on the downstream diff --git a/conformance/conformance.go b/conformance/conformance.go index 252dea5e9b..d074f16956 100644 --- a/conformance/conformance.go +++ b/conformance/conformance.go @@ -96,6 +96,7 @@ func DefaultOptions(t *testing.T) suite.ConformanceOptions { SkipTests: skipTests, SupportedFeatures: supportedFeatures, TimeoutConfig: conformanceconfig.DefaultTimeoutConfig(), + SkipProvisionalTests: *flags.SkipProvisionalTests, } } diff --git a/conformance/tests/udproute-simple.go b/conformance/tests/udproute-simple.go index e8c84b86e7..8945d3fead 100644 --- a/conformance/tests/udproute-simple.go +++ b/conformance/tests/udproute-simple.go @@ -42,6 +42,7 @@ var UDPRouteTest = suite.ConformanceTest{ features.SupportUDPRoute, features.SupportGateway, }, + Provisional: true, Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { t.Run("Simple UDP request matching UDPRoute should reach coredns backend", func(t *testing.T) { namespace := "gateway-conformance-infra" diff --git a/conformance/utils/flags/flags.go b/conformance/utils/flags/flags.go index f97eeba044..95da9eaad1 100644 --- a/conformance/utils/flags/flags.go +++ b/conformance/utils/flags/flags.go @@ -48,4 +48,5 @@ var ( AllowCRDsMismatch = flag.Bool("allow-crds-mismatch", false, "Flag to allow the suite not to fail in case there is a mismatch between CRDs versions and channels.") ConformanceProfiles = flag.String("conformance-profiles", "", "Comma-separated list of the conformance profiles to run") ReportOutput = flag.String("report-output", "", "The file where to write the conformance report") + SkipProvisionalTests = flag.Bool("skip-provisional-tests", false, "Whether to skip provisional tests") ) diff --git a/conformance/utils/suite/conformance.go b/conformance/utils/suite/conformance.go index 7274f8d033..3bbbbc1c23 100644 --- a/conformance/utils/suite/conformance.go +++ b/conformance/utils/suite/conformance.go @@ -36,6 +36,7 @@ type ConformanceTest struct { Slow bool Parallel bool Test func(*testing.T, *ConformanceTestSuite) + Provisional bool } // Run runs an individual tests, applying and cleaning up the required manifests diff --git a/conformance/utils/suite/reports.go b/conformance/utils/suite/reports.go index ea1e61a742..4b03d67733 100644 --- a/conformance/utils/suite/reports.go +++ b/conformance/utils/suite/reports.go @@ -18,7 +18,6 @@ package suite import ( "fmt" - "sort" "k8s.io/apimachinery/pkg/util/sets" @@ -38,10 +37,11 @@ type testResult struct { type resultType string var ( - testSucceeded resultType = "SUCCEEDED" - testFailed resultType = "FAILED" - testSkipped resultType = "SKIPPED" - testNotSupported resultType = "NOT_SUPPORTED" + testSucceeded resultType = "SUCCEEDED" + testFailed resultType = "FAILED" + testSkipped resultType = "SKIPPED" + testNotSupported resultType = "NOT_SUPPORTED" + testProvisionalSkipped resultType = "PROVISIONAL_SKIPPED" ) type profileReportsMap map[ConformanceProfileName]confv1.ProfileReport @@ -136,10 +136,7 @@ func (p profileReportsMap) compileResults(supportedFeaturesMap map[ConformancePr supportedFeatures := supportedFeaturesMap[ConformanceProfileName(report.Name)] if report.Extended != nil { if supportedFeatures != nil { - supportedFeatures := supportedFeatures.UnsortedList() - sort.Slice(supportedFeatures, func(i, j int) bool { - return supportedFeatures[i] < supportedFeatures[j] - }) + supportedFeatures := sets.List(supportedFeatures) for _, f := range supportedFeatures { report.Extended.SupportedFeatures = append(report.Extended.SupportedFeatures, string(f)) } @@ -149,10 +146,7 @@ func (p profileReportsMap) compileResults(supportedFeaturesMap map[ConformancePr unsupportedFeatures := unsupportedFeaturesMap[ConformanceProfileName(report.Name)] if report.Extended != nil { if unsupportedFeatures != nil { - unsupportedFeatures := unsupportedFeatures.UnsortedList() - sort.Slice(unsupportedFeatures, func(i, j int) bool { - return unsupportedFeatures[i] < unsupportedFeatures[j] - }) + unsupportedFeatures := sets.List(unsupportedFeatures) for _, f := range unsupportedFeatures { report.Extended.UnsupportedFeatures = append(report.Extended.UnsupportedFeatures, string(f)) } diff --git a/conformance/utils/suite/suite.go b/conformance/utils/suite/suite.go index 279298f2d0..e4277c2f14 100644 --- a/conformance/utils/suite/suite.go +++ b/conformance/utils/suite/suite.go @@ -71,6 +71,7 @@ type ConformanceTestSuite struct { SupportedFeatures sets.Set[features.FeatureName] TimeoutConfig config.TimeoutConfig SkipTests sets.Set[string] + SkipProvisionalTests bool RunTest string ManifestFS []fs.FS UsableNetworkAddresses []v1beta1.GatewayAddress @@ -144,6 +145,8 @@ type ConformanceOptions struct { // SkipTests contains all the tests not to be run and can be used to opt out // of specific tests SkipTests []string + // SkipProvisionalTests indicates whether or not to skip provisional tests. + SkipProvisionalTests bool // RunTest is a single test to run, mostly for development/debugging convenience. RunTest string @@ -248,6 +251,7 @@ func NewConformanceTestSuite(options ConformanceOptions) (*ConformanceTestSuite, TimeoutConfig: options.TimeoutConfig, SkipTests: sets.New(options.SkipTests...), RunTest: options.RunTest, + SkipProvisionalTests: options.SkipProvisionalTests, ManifestFS: options.ManifestFS, UsableNetworkAddresses: options.UsableNetworkAddresses, UnusableNetworkAddresses: options.UnusableNetworkAddresses, @@ -431,6 +435,9 @@ func (suite *ConformanceTestSuite) Run(t *testing.T, tests []ConformanceTest) er if suite.SkipTests.Has(test.ShortName) { res = testSkipped } + if suite.SkipProvisionalTests && test.Provisional { + res = testProvisionalSkipped + } if !suite.SupportedFeatures.HasAll(test.Features...) { res = testNotSupported } @@ -486,16 +493,27 @@ func (suite *ConformanceTestSuite) Report() (*confv1.ConformanceReport, error) { } sort.Strings(testNames) profileReports := newReports() + succeededProvisionalTestSet := sets.Set[string]{} for _, tN := range testNames { - testResult := suite.results[tN] - conformanceProfiles := getConformanceProfilesForTest(testResult.test, suite.conformanceProfiles).UnsortedList() + tr := suite.results[tN] + if tr.result == testProvisionalSkipped { + continue + } + if tr.result == testSucceeded && tr.test.Provisional { + succeededProvisionalTestSet.Insert(tN) + } + conformanceProfiles := getConformanceProfilesForTest(tr.test, suite.conformanceProfiles).UnsortedList() sort.Slice(conformanceProfiles, func(i, j int) bool { return conformanceProfiles[i].Name < conformanceProfiles[j].Name }) for _, profile := range conformanceProfiles { - profileReports.addTestResults(*profile, testResult) + profileReports.addTestResults(*profile, tr) } } + var succeededProvisionalTests []string + if len(succeededProvisionalTestSet) > 0 { + succeededProvisionalTests = sets.List(succeededProvisionalTestSet) + } profileReports.compileResults(suite.extendedSupportedFeatures, suite.extendedUnsupportedFeatures) @@ -504,12 +522,13 @@ func (suite *ConformanceTestSuite) Report() (*confv1.ConformanceReport, error) { APIVersion: confv1.GroupVersion.String(), Kind: "ConformanceReport", }, - Date: time.Now().Format(time.RFC3339), - Mode: suite.mode, - Implementation: suite.implementation, - GatewayAPIVersion: suite.apiVersion, - GatewayAPIChannel: suite.apiChannel, - ProfileReports: profileReports.list(), + Date: time.Now().Format(time.RFC3339), + Mode: suite.mode, + Implementation: suite.implementation, + GatewayAPIVersion: suite.apiVersion, + GatewayAPIChannel: suite.apiChannel, + ProfileReports: profileReports.list(), + SucceededProvisionalTests: succeededProvisionalTests, }, nil } diff --git a/conformance/utils/suite/suite_test.go b/conformance/utils/suite/suite_test.go index 58be60bec0..9afb53878c 100644 --- a/conformance/utils/suite/suite_test.go +++ b/conformance/utils/suite/suite_test.go @@ -23,8 +23,11 @@ import ( "github.com/stretchr/testify/assert" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" + confv1 "sigs.k8s.io/gateway-api/conformance/apis/v1" "sigs.k8s.io/gateway-api/pkg/consts" + "sigs.k8s.io/gateway-api/pkg/features" ) func TestGetAPIVersionAndChannel(t *testing.T) { @@ -176,3 +179,231 @@ func TestGetAPIVersionAndChannel(t *testing.T) { }) } } + +const ( + coreFeature features.FeatureName = "coreFature" + extendedFeature features.FeatureName = "extendedFeature" + + testProfileName ConformanceProfileName = "testProfile" +) + +var testProfile = ConformanceProfile{ + Name: "testProfile", + CoreFeatures: sets.New(coreFeature), + ExtendedFeatures: sets.New(extendedFeature), +} + +var ( + coreTest = ConformanceTest{ + ShortName: "coreTest", + Features: []features.FeatureName{coreFeature}, + } + extendedTest = ConformanceTest{ + ShortName: "extendedTest", + Features: []features.FeatureName{extendedFeature}, + } + coreProvisionalTest = ConformanceTest{ + ShortName: "coreProvisionalTest", + Features: []features.FeatureName{coreFeature}, + Provisional: true, + } + extendedProvisionalTest = ConformanceTest{ + ShortName: "extendedProvisionalTest", + Features: []features.FeatureName{extendedFeature}, + Provisional: true, + } +) + +func TestSuiteReport(t *testing.T) { + testCases := []struct { + name string + features sets.Set[features.FeatureName] + extendedSupportedFeatures map[ConformanceProfileName]sets.Set[features.FeatureName] + profiles sets.Set[ConformanceProfileName] + skipProvisionalTests bool + results map[string]testResult + expectedReport confv1.ConformanceReport + expectedError error + }{ + { + name: "all tests succeeded", + features: sets.New(coreFeature, extendedFeature), + extendedSupportedFeatures: map[ConformanceProfileName]sets.Set[features.FeatureName]{ + testProfileName: sets.New(extendedFeature), + }, + profiles: sets.New(testProfileName), + results: map[string]testResult{ + coreTest.ShortName: { + result: testSucceeded, + test: coreTest, + }, + extendedTest.ShortName: { + result: testSucceeded, + test: extendedTest, + }, + coreProvisionalTest.ShortName: { + result: testSucceeded, + test: coreProvisionalTest, + }, + extendedProvisionalTest.ShortName: { + result: testSucceeded, + test: extendedProvisionalTest, + }, + }, + expectedReport: confv1.ConformanceReport{ + ProfileReports: []confv1.ProfileReport{ + { + Name: string(testProfileName), + Summary: "Core tests succeeded. Extended tests succeeded.", + Core: confv1.Status{ + Result: confv1.Success, + Statistics: confv1.Statistics{ + Passed: 2, + }, + }, + Extended: &confv1.ExtendedStatus{ + Status: confv1.Status{ + Result: confv1.Success, + Statistics: confv1.Statistics{ + Passed: 2, + }, + }, + SupportedFeatures: []string{string(extendedFeature)}, + }, + }, + }, + SucceededProvisionalTests: []string{ + coreProvisionalTest.ShortName, + extendedProvisionalTest.ShortName, + }, + }, + }, + { + name: "mixed results", + features: sets.New(coreFeature, extendedFeature), + extendedSupportedFeatures: map[ConformanceProfileName]sets.Set[features.FeatureName]{ + testProfileName: sets.New(extendedFeature), + }, + profiles: sets.New(testProfileName), + results: map[string]testResult{ + coreTest.ShortName: { + result: testFailed, + test: coreTest, + }, + extendedTest.ShortName: { + result: testSkipped, + test: extendedTest, + }, + coreProvisionalTest.ShortName: { + result: testSucceeded, + test: coreProvisionalTest, + }, + extendedProvisionalTest.ShortName: { + result: testProvisionalSkipped, + test: extendedProvisionalTest, + }, + }, + expectedReport: confv1.ConformanceReport{ + ProfileReports: []confv1.ProfileReport{ + { + Name: string(testProfileName), + Summary: "Core tests failed with 1 test failures. Extended tests partially succeeded with 1 test skips.", + Core: confv1.Status{ + Result: confv1.Failure, + Statistics: confv1.Statistics{ + Passed: 1, + Failed: 1, + }, + FailedTests: []string{ + coreTest.ShortName, + }, + }, + Extended: &confv1.ExtendedStatus{ + Status: confv1.Status{ + Result: confv1.Partial, + Statistics: confv1.Statistics{ + Skipped: 1, + }, + SkippedTests: []string{ + extendedTest.ShortName, + }, + }, + SupportedFeatures: []string{string(extendedFeature)}, + }, + }, + }, + SucceededProvisionalTests: []string{ + coreProvisionalTest.ShortName, + }, + }, + }, + { + name: "skip provisional tests", + features: sets.New(coreFeature, extendedFeature), + extendedSupportedFeatures: map[ConformanceProfileName]sets.Set[features.FeatureName]{ + testProfileName: sets.New(extendedFeature), + }, + profiles: sets.New(testProfileName), + skipProvisionalTests: true, + results: map[string]testResult{ + coreTest.ShortName: { + result: testSucceeded, + test: coreTest, + }, + extendedTest.ShortName: { + result: testSucceeded, + test: extendedTest, + }, + coreProvisionalTest.ShortName: { + result: testProvisionalSkipped, + test: coreProvisionalTest, + }, + extendedProvisionalTest.ShortName: { + result: testProvisionalSkipped, + test: extendedProvisionalTest, + }, + }, + expectedReport: confv1.ConformanceReport{ + ProfileReports: []confv1.ProfileReport{ + { + Name: string(testProfileName), + Summary: "Core tests succeeded. Extended tests succeeded.", + Core: confv1.Status{ + Result: confv1.Success, + Statistics: confv1.Statistics{ + Passed: 1, + }, + }, + Extended: &confv1.ExtendedStatus{ + Status: confv1.Status{ + Result: confv1.Success, + Statistics: confv1.Statistics{ + Passed: 1, + }, + }, + SupportedFeatures: []string{string(extendedFeature)}, + }, + }, + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + conformanceProfileMap[testProfileName] = testProfile + + suite := ConformanceTestSuite{ + conformanceProfiles: tc.profiles, + SupportedFeatures: tc.features, + extendedSupportedFeatures: tc.extendedSupportedFeatures, + results: tc.results, + SkipProvisionalTests: tc.skipProvisionalTests, + } + report, err := suite.Report() + assert.Equal(t, tc.expectedReport.ProfileReports, report.ProfileReports) + assert.Equal(t, tc.expectedReport.SucceededProvisionalTests, report.SucceededProvisionalTests) + assert.Equal(t, tc.expectedError, err) + }) + } +}