Skip to content

fix(config-brancher): preserve presubmit tests when using --skip-periodics #4837

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.

Sign up for GitHub

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

Open
petr-muller wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(config-brancher): preserve presubmit tests when using --skip-periodics #4837

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 17 additions & 3 deletions cmd/config-brancher/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,11 +154,25 @@ func generateBranchedConfigs(currentRelease, bumpRelease string, futureReleases
return output
}

// removePeriodics removes periodic tests from the configuration
// removePeriodics removes periodic tests from the configuration.
// Tests that are both periodic and presubmit (have Presubmit: true) are preserved
// but have their periodic-related fields (Cron, Interval, MinimumInterval, ReleaseController)
// and Presubmit field removed.
func removePeriodics(tests *[]api.TestStepConfiguration) {
for i := len(*tests) - 1; i >= 0; i-- {
if !(*tests)[i].Portable && (*tests)[i].IsPeriodic() {
*tests = append((*tests)[:i], (*tests)[i+1:]...)
test := &(*tests)[i]
if !test.Portable && test.IsPeriodic() {
// If the test is also a presubmit, keep it but remove periodic fields
if test.Presubmit {
test.Cron = nil
test.Interval = nil
test.MinimumInterval = nil
test.ReleaseController = false
test.Presubmit = false
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting Presubmit to false defeats the purpose of preserving presubmit tests. According to the PR description, these tests should continue to exist as presubmit jobs on the destination branch. This line should be removed to keep Presubmit: true.

Suggested change
test.Presubmit = false

Copilot uses AI. Check for mistakes.
Copy link
Member Author

@petr-muller petr-muller Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This stanza is only used when the job would otherwise be generated only as a periodic. Without any special stanzas, generating a presubmit job is the default behavior that does not need to be expliitly enabled.

} else {
// Otherwise, remove the test entirely
*tests = append((*tests)[:i], (*tests)[i+1:]...)
}
}
}
}
Expand Down
43 changes: 43 additions & 0 deletions cmd/config-brancher/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,49 @@ func TestGenerateBranchedConfigs(t *testing.T) {
},
},
},
{
name: "config with tests that are both periodic and presubmit gets branched with presubmit preserved but periodic fields removed when skip-periodics is set",
currentRelease: "current-release",
futureReleases: []string{"future-release"},
skipPeriodics: true,
input: config.DataWithInfo{
Configuration: api.ReleaseBuildConfiguration{
Tests: []api.TestStepConfiguration{
{As: "periodic-only", Cron: &cron},
{As: "periodic-and-presubmit", Cron: &cron, Presubmit: true},
{As: "periodic-presubmit-interval", Interval: &interval, Presubmit: true},
},
PromotionConfiguration: &api.PromotionConfiguration{
Targets: []api.PromotionTarget{{
Name: "current-release",
Namespace: "ocp",
}},
},
},
Info: config.Info{
Metadata: api.Metadata{Org: "org", Repo: "repo", Branch: "master"},
},
},
output: []config.DataWithInfo{
{
Configuration: api.ReleaseBuildConfiguration{
Tests: []api.TestStepConfiguration{
{As: "periodic-and-presubmit"},
{As: "periodic-presubmit-interval"},
},
PromotionConfiguration: &api.PromotionConfiguration{
Targets: []api.PromotionTarget{{
Name: "future-release",
Namespace: "ocp",
}},
},
},
Info: config.Info{
Metadata: api.Metadata{Org: "org", Repo: "repo", Branch: "release-future-release"},
},
},
},
},
{
name: "previously branched config that promotes to the current release from master bumps to the future release and de-mirrors correctly",
currentRelease: "current-release",
Expand Down