Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

💝 Dropping redundant variant suffix from test names #414

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ require (
github.com/openshift/ci-tools v0.0.0-20240910200445-d561bdd430ec
github.com/operator-framework/api v0.27.0
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.9.0
go.uber.org/zap v1.27.0
golang.org/x/mod v0.20.0
golang.org/x/sync v0.7.0
Expand Down Expand Up @@ -92,6 +93,7 @@ require (
github.com/openshift/api v0.0.0-20240522145529-93d6bda14341 // indirect
github.com/peterbourgon/diskv v2.0.1+incompatible // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
github.com/prometheus/client_golang v1.19.1 // indirect
github.com/prometheus/client_model v0.6.1 // indirect
github.com/prometheus/common v0.55.0 // indirect
Expand Down
2 changes: 1 addition & 1 deletion pkg/prowgen/prowgen.go
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ func AlwaysRunInjector() JobConfigInjector {
}

for _, t := range tests {
name := ToName(*r, &t, ocpVersion)
name := ToName(*r, &t)
if (t.OnDemand || t.RunIfChanged != "" || onDemandForOpenShift) && strings.Contains(jobConfig.PresubmitsStatic[k][i].Name, name) {
jobConfig.PresubmitsStatic[k][i].AlwaysRun = false
}
Expand Down
24 changes: 9 additions & 15 deletions pkg/prowgen/prowgen_command_name_mapping.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package prowgen

import (
"fmt"
"log"
"strings"
)
Expand All @@ -13,25 +12,20 @@ const (

// ToName creates a test name for the given Test following the constraints in openshift/release.
// - name cannot be longer than maxNameLength characters.
func ToName(r Repository, test *Test, openShiftVersion string) string {

variant := strings.ReplaceAll(openShiftVersion, ".", "")
suffix := fmt.Sprintf("-aws-%s", variant)
func ToName(r Repository, test *Test) string {
continuousSuffix := "-c"

maxCommandLength := maxNameLength - len(suffix) - len(continuousSuffix)
maxCommandLength := maxNameLength - len(continuousSuffix)
if len(test.Command) > maxCommandLength {
sha := test.HexSha() // guarantees uniqueness
prefix := test.Command[:maxCommandLength-len(sha)-1]
if strings.HasSuffix(prefix, "-") {
// OpenShift CI doesnt' like double dashes, such as `stable-latest-test-kafka--7465737-aws-ocp-412`.
// So, if the prefix of the command ends with a dash, we remove it.
prefix = prefix[:len(prefix)-1]
}
// OpenShift CI doesnt' like double dashes, such as `stable-latest-test-kafka--7465737-aws-ocp-412`.
// So, if the prefix of the command ends with a dash, we remove it.
prefix = strings.TrimSuffix(prefix, "-")
newTarget := prefix + "-" + sha
log.Println(r.RepositoryDirectory(), "command as test name is too long", test.Command, "truncating it to", newTarget)
return fmt.Sprintf("%s%s", newTarget, suffix)
log.Println(r.RepositoryDirectory(), "command as test name is too long",
test.Command, "truncating it to", newTarget)
return newTarget
}

return fmt.Sprintf("%s%s", test.Command, suffix)
return test.Command
}
88 changes: 40 additions & 48 deletions pkg/prowgen/prowgen_command_name_mapping_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@ import (
"fmt"
"strings"
"testing"

"github.com/stretchr/testify/assert"
)

func TestToName(t *testing.T) {

openshiftVersion := "4.11"
suffix := "-aws-411"
continuousSuffix := "-c"

tests := []struct {
Expand All @@ -18,63 +19,54 @@ func TestToName(t *testing.T) {
test *Test
openShiftVersion string
want string
}{
{
name: fmt.Sprintf("%d length name", maxNameLength),
r: Repository{},
test: &Test{
Command: strings.Repeat("a", maxNameLength),
},
openShiftVersion: openshiftVersion,
want: fmt.Sprintf("%s-%s%s", strings.Repeat("a", maxNameLength-len(suffix)-len(continuousSuffix)-shaLength-1) /* hex sha1 */, "32e067e", suffix),
}{{
name: "max length name",
r: Repository{},
test: &Test{
Command: strings.Repeat("a", maxNameLength),
},
{
name: fmt.Sprintf("%d length name", maxNameLength-len(suffix)-len(continuousSuffix)+1),
r: Repository{},
test: &Test{
Command: strings.Repeat("a", maxNameLength-len(suffix)-len(continuousSuffix)+1),
},
openShiftVersion: openshiftVersion,
want: fmt.Sprintf("%s-%s%s", strings.Repeat("a", maxNameLength-len(suffix)-len(continuousSuffix)-shaLength-1) /* hex sha1 */, "2368a1a", suffix),
openShiftVersion: openshiftVersion,
want: fmt.Sprintf("%s-%s", strings.Repeat("a", maxNameLength-len(continuousSuffix)-shaLength-1) /* hex sha1 */, "32e067e"),
}, {
name: "max length name without continuous +1",
r: Repository{},
test: &Test{
Command: strings.Repeat("a", maxNameLength-len(continuousSuffix)+1),
},
{
name: fmt.Sprintf("%d length name", maxNameLength-len(suffix)-len(continuousSuffix)),
r: Repository{},
test: &Test{
Command: strings.Repeat("a", maxNameLength-len(suffix)-len(continuousSuffix)),
},
openShiftVersion: openshiftVersion,
want: fmt.Sprintf("%s%s", strings.Repeat("a", maxNameLength-len(suffix)-len(continuousSuffix)), suffix),
openShiftVersion: openshiftVersion,
want: fmt.Sprintf("%s-%s", strings.Repeat("a", maxNameLength-len(continuousSuffix)-shaLength-1) /* hex sha1 */, "52cedd6"),
}, {
name: "max length name without continuous",
r: Repository{},
test: &Test{
Command: strings.Repeat("a", maxNameLength-len(continuousSuffix)),
},
{
name: "test-conformance name",
r: Repository{},
test: &Test{
Command: "test-conformance",
},
openShiftVersion: openshiftVersion,
want: fmt.Sprintf("%s%s", "test-conformance", suffix),
openShiftVersion: openshiftVersion,
want: strings.Repeat("a", maxNameLength-len(continuousSuffix)),
}, {
name: "test-conformance name",
r: Repository{},
test: &Test{
Command: "test-conformance",
},
{
name: "test-kafka-broker-upstream-nightly",
r: Repository{},
test: &Test{
Command: "test-kafka-broker-upstream-nightly",
},
openShiftVersion: openshiftVersion,
want: fmt.Sprintf("%s%s", "test-kafka-broker-upstre-fbbddbf", suffix),
openShiftVersion: openshiftVersion,
want: "test-conformance",
}, {
name: "test-kafka-broker-filter-upstream-nightly",
r: Repository{},
test: &Test{
Command: "test-kafka-broker-filter-upstream-nightly",
},
}
openShiftVersion: openshiftVersion,
want: "test-kafka-broker-filter-upstrea-b7f30b5",
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if len(tt.want) > maxNameLength-len(continuousSuffix) {
t.Fatalf("Test misconfiguration want cannot be longer than %d, got %d", maxNameLength-len(continuousSuffix), len(tt.want))
}
got := ToName(tt.r, tt.test, tt.openShiftVersion)
if got != tt.want {
t.Errorf("ToName() = %v (length %d), want %v (length %d)", got, len(got), tt.want, len(tt.want))
}
t.Logf("ToName() = %v (length %d), want %v (length %d)", got, len(got), tt.want, len(tt.want))
got := ToName(tt.r, tt.test)
assert.Equal(t, tt.want, got)
})
}
}
2 changes: 1 addition & 1 deletion pkg/prowgen/prowgen_tests.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func DiscoverTests(r Repository, openShift OpenShift, sourceImageName string, sk

for i := range tests {
test := &tests[i]
as := ToName(r, test, openShift.Version)
as := ToName(r, test)

var testTimeout *prowapi.Duration
var jobTimeout *prowapi.Duration
Expand Down
35 changes: 17 additions & 18 deletions pkg/prowgen/prowgen_tests_discovery_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package prowgen
import (
"fmt"
"math/rand"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -99,7 +98,7 @@ func TestDiscoverTestsServing(t *testing.T) {

expectedTests := []cioperatorapi.TestStepConfiguration{
{
As: "perf-tests-aws-412",
As: "perf-tests",
MultiStageTestConfiguration: &cioperatorapi.MultiStageTestConfiguration{
ClusterProfile: serverlessClusterProfile,
Test: []cioperatorapi.TestStep{
Expand All @@ -123,7 +122,7 @@ func TestDiscoverTestsServing(t *testing.T) {
},
},
{
As: "test-e2e-aws-412",
As: "test-e2e",
Optional: true,
MultiStageTestConfiguration: &cioperatorapi.MultiStageTestConfiguration{
ClusterProfile: serverlessClusterProfile,
Expand All @@ -148,7 +147,7 @@ func TestDiscoverTestsServing(t *testing.T) {
},
},
{
As: "test-e2e-aws-412-c",
As: "test-e2e-c",
Cron: cron,
MultiStageTestConfiguration: &cioperatorapi.MultiStageTestConfiguration{
ClusterProfile: serverlessClusterProfile,
Expand All @@ -173,7 +172,7 @@ func TestDiscoverTestsServing(t *testing.T) {
},
},
{
As: "test-e2e-tls-aws-412",
As: "test-e2e-tls",
MultiStageTestConfiguration: &cioperatorapi.MultiStageTestConfiguration{
ClusterProfile: serverlessClusterProfile,
Test: []cioperatorapi.TestStep{
Expand All @@ -197,7 +196,7 @@ func TestDiscoverTestsServing(t *testing.T) {
},
},
{
As: "test-e2e-tls-aws-412-c",
As: "test-e2e-tls-c",
Cron: cron,
MultiStageTestConfiguration: &cioperatorapi.MultiStageTestConfiguration{
ClusterProfile: serverlessClusterProfile,
Expand All @@ -222,7 +221,7 @@ func TestDiscoverTestsServing(t *testing.T) {
},
},
{
As: "ui-e2e-aws-412",
As: "ui-e2e",
RunIfChanged: "test/ui",
MultiStageTestConfiguration: &cioperatorapi.MultiStageTestConfiguration{
ClusterProfile: serverlessClusterProfile,
Expand Down Expand Up @@ -331,7 +330,7 @@ func TestDiscoverTestsServingClusterClaim(t *testing.T) {

expectedTests := []cioperatorapi.TestStepConfiguration{
{
As: fmt.Sprintf("perf-tests-aws-%s", strings.ReplaceAll("4.16", ".", "")),
As: "perf-tests",
ClusterClaim: &cioperatorapi.ClusterClaim{
Product: cioperatorapi.ReleaseProductOCP,
Version: "4.16",
Expand Down Expand Up @@ -438,7 +437,7 @@ func TestDiscoverTestsEventing(t *testing.T) {

expectedTests := []cioperatorapi.TestStepConfiguration{
{
As: "test-conformance-aws-412",
As: "test-conformance",
MultiStageTestConfiguration: &cioperatorapi.MultiStageTestConfiguration{
ClusterProfile: serverlessClusterProfile,
Test: []cioperatorapi.TestStep{
Expand All @@ -462,7 +461,7 @@ func TestDiscoverTestsEventing(t *testing.T) {
},
},
{
As: "test-conformance-aws-412-c",
As: "test-conformance-c",
Cron: pointer.String("23 1 * * 2,6"),
MultiStageTestConfiguration: &cioperatorapi.MultiStageTestConfiguration{
ClusterProfile: serverlessClusterProfile,
Expand All @@ -487,15 +486,15 @@ func TestDiscoverTestsEventing(t *testing.T) {
},
},
{
As: "test-conformance-long-lo-510e96a-aws-412",
As: "test-conformance-long-long-long-80ea36d",
MultiStageTestConfiguration: &cioperatorapi.MultiStageTestConfiguration{
ClusterProfile: serverlessClusterProfile,
Test: []cioperatorapi.TestStep{
{
LiteralTestStep: &cioperatorapi.LiteralTestStep{
As: "test",
From: eventingSourceImage,
Commands: formatCommand("make test-conformance-long-long-long-command"),
Commands: formatCommand("make test-conformance-long-long-long-long-long-command"),
Resources: cioperatorapi.ResourceRequirements{
Requests: cioperatorapi.ResourceList{
"cpu": "100m",
Expand All @@ -511,7 +510,7 @@ func TestDiscoverTestsEventing(t *testing.T) {
},
},
{
As: "test-conformance-long-lo-510e96a-aws-412-c",
As: "test-conformance-long-long-long-80ea36d-c",
Cron: pointer.String("43 1 * * 2,6"),
MultiStageTestConfiguration: &cioperatorapi.MultiStageTestConfiguration{
ClusterProfile: serverlessClusterProfile,
Expand All @@ -520,7 +519,7 @@ func TestDiscoverTestsEventing(t *testing.T) {
LiteralTestStep: &cioperatorapi.LiteralTestStep{
As: "test",
From: eventingSourceImage,
Commands: formatCommand("make test-conformance-long-long-long-command"),
Commands: formatCommand("make test-conformance-long-long-long-long-long-command"),
Resources: cioperatorapi.ResourceRequirements{
Requests: cioperatorapi.ResourceList{
"cpu": "100m",
Expand All @@ -536,7 +535,7 @@ func TestDiscoverTestsEventing(t *testing.T) {
},
},
{
As: "test-e2e-aws-412",
As: "test-e2e",
MultiStageTestConfiguration: &cioperatorapi.MultiStageTestConfiguration{
ClusterProfile: serverlessClusterProfile,
Test: []cioperatorapi.TestStep{
Expand All @@ -560,7 +559,7 @@ func TestDiscoverTestsEventing(t *testing.T) {
},
},
{
As: "test-e2e-aws-412-c",
As: "test-e2e-c",
Cron: pointer.String("4 1 * * 2,6"),
MultiStageTestConfiguration: &cioperatorapi.MultiStageTestConfiguration{
ClusterProfile: serverlessClusterProfile,
Expand All @@ -585,7 +584,7 @@ func TestDiscoverTestsEventing(t *testing.T) {
},
},
{
As: "test-reconciler-aws-412",
As: "test-reconciler",
MultiStageTestConfiguration: &cioperatorapi.MultiStageTestConfiguration{
ClusterProfile: serverlessClusterProfile,
Test: []cioperatorapi.TestStep{
Expand All @@ -609,7 +608,7 @@ func TestDiscoverTestsEventing(t *testing.T) {
},
},
{
As: "test-reconciler-aws-412-c",
As: "test-reconciler-c",
Cron: pointer.String("16 5 * * 2,6"),
MultiStageTestConfiguration: &cioperatorapi.MultiStageTestConfiguration{
ClusterProfile: serverlessClusterProfile,
Expand Down
2 changes: 1 addition & 1 deletion pkg/prowgen/testdata/eventing/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@ test-conformance:
test-e2e:
echo "Hello"

test-conformance-long-long-long-command:
test-conformance-long-long-long-long-long-command:
echo "Hello"
27 changes: 27 additions & 0 deletions vendor/github.com/pmezard/go-difflib/LICENSE

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading