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

[petr-muller]

The --skip-periodics option was incorrectly removing test definitions that were both periodic and presubmit (tests with both a cron/interval stanza and presubmit: true). This fix modifies the removePeriodics function to preserve these tests by removing only the periodic-related fields (Cron, Interval, MinimumInterval, ReleaseController, Presubmit)
while keeping the test definition itself, so it continues to exist as a presubmit job on the destination branch, and we do not leave such presubmit behind when we branch.

xref: openshift/release#70913 (comment)

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

[openshift-ci-robot]

Pipeline controller notification
This repository is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. Review these jobs and use /test <job> to manually trigger optional jobs most likely to be impacted by the proposed changes.

[coderabbitai]

Walkthrough

Extended the removePeriodics function in config-brancher to conditionally preserve tests that are both periodic and presubmit by stripping periodic-related fields and clearing the Presubmit flag, while removing purely periodic tests. Added a corresponding test case to verify this behavior.

Changes

Cohort / File(s)	Summary
Periodic test handling logic and tests cmd/config-brancher/main.go, cmd/config-brancher/main_test.go	Modified removePeriodics to preserve tests that have both Presubmit and periodic properties by stripping periodic fields (Cron, Interval, MinimumInterval, ReleaseController) and clearing Presubmit, while removing non-portable periodic tests. Added a new test case to verify presubmit tests are preserved and periodic-only tests are removed when skip-periodics is enabled.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Conditional logic verification: Ensure the logic correctly distinguishes between tests that are both periodic and presubmit versus purely periodic tests, and that the right fields are being cleared
• Test case coverage: Verify the new test case covers the critical scenarios and that the expected behavior (preserved presubmit tests, removed periodic-only tests, promoted target changes) is correct
• Field correctness: Confirm that the four fields being stripped (Cron, Interval, MinimumInterval, ReleaseController) represent the complete set of periodic-related attributes

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 472c375 and 33202fd.

📒 Files selected for processing (2)

• cmd/config-brancher/main.go (1 hunks)
• cmd/config-brancher/main_test.go (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

• cmd/config-brancher/main_test.go
• cmd/config-brancher/main.go

🔇 Additional comments (2)

cmd/config-brancher/main.go (1)

157-178: LGTM! Well-implemented conditional preservation logic.

The updated removePeriodics function correctly preserves tests that are both periodic and presubmit by stripping only the periodic-related fields while keeping the test definition itself. The backward iteration ensures safe slice modification during removal, and the logic aligns perfectly with the test expectations.

cmd/config-brancher/main_test.go (1)

378-420: LGTM! Comprehensive test coverage for the new behavior.

The test case thoroughly validates the conditional preservation logic: periodic-only tests are removed, while tests marked as both periodic and presubmit are preserved with their periodic fields stripped. The expected output correctly reflects the behavior implemented in removePeriodics.

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

• Provide your own instructions using the high_level_summary_instructions setting.
• Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
• Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
4. 📊 Contributor Summary — Include a Markdown table showing contributions:
   | Contributor | Lines Added | Lines Removed | Files Changed |
5. ✔️ Additional Notes — Add any extra reviewer context.
   Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull Request Overview

This PR fixes a bug in the config-brancher tool where the --skip-periodics option was incorrectly removing test definitions that served dual purposes (both periodic and presubmit). The fix ensures these tests are preserved as presubmit jobs on the destination branch by removing only their periodic-related fields.

Key Changes:

• Modified removePeriodics() function to preserve tests that have both periodic and presubmit characteristics
• Added comprehensive test coverage for the new branching behavior with dual-purpose tests

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
cmd/config-brancher/main.go	Updated removePeriodics() to preserve presubmit tests by clearing only periodic fields instead of removing the entire test definition
cmd/config-brancher/main_test.go	Added test case validating that dual-purpose tests are correctly preserved as presubmit-only tests when branching with --skip-periodics

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

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

[petr-muller]

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.

[openshift-ci-robot]

Scheduling required tests:
/test e2e

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test integration-optional-test

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: petr-muller, Prucek

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [Prucek]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[petr-muller]

/hold

I'd like to see what breaking-changes shows. I expect this to add some jobs to o/release that were previously skipped.

[openshift-ci]

@petr-muller: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/breaking-changes	33202fd	link	false	/test breaking-changes
ci/prow/images	33202fd	link	true	/test images

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.