fix: propagate verification failures instead of silently swallowing them#1711
Merged
jonesbusy merged 3 commits intojenkins-infra:mainfrom Apr 18, 2026
Merged
Conversation
In dry-run mode, the tool was still writing modernization metadata files to disk and attempting to push them to the metadata repository. This conflicts with the expected non-destructive behavior of dry-run. Changes: - Add !config.isDryRun() check in the finally block of process() - Skip collectModernizationMetadata() and related operations in dry-run - Only plugin-metadata.json (needed for processing) is now created Before fix: - modernization-metadata.json was created in dry-run - metadata-plugin-modernizer/* files were created After fix: - modernization-metadata.json is NOT created in dry-run - No metadata repository operations in dry-run mode Bug: jenkins-infra#2 - Dry-run still writes metadata
verifyPlugin() called plugin.withoutErrors() twice — once inside the failure branch and once unconditionally — so build failures after recipe application were always silently discarded. The guard in start() that was meant to abort the run and skip the PR on broken builds never triggered. Remove both withoutErrors() calls so errors from verify() propagate back to start(), which already has the correct skip logic. Upgrade the log from INFO to WARN to match the severity.
…ails In run mode, a verification failure after applying a recipe must prevent commits and PRs so broken code is never pushed. In dry-run mode the user is only previewing changes so a verification failure should be logged as a warning but must not suppress the diff output.
jonesbusy
approved these changes
Apr 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
verifyPlugin()calledplugin.withoutErrors()unconditionally at the end, regardless of whether the build actually passed. The caller instart()would always see a clean plugin and continue to fork, commit, and open a PR — even when the verification build failed. Removed bothwithoutErrors()calls and upgraded the log from INFO to WARN.In dry-run mode, the error is still cleared after logging so the diff preview is still shown to the user (that's intentional — you're previewing, not committing).
Testing Done
I tested it locally, and here is the before and after
Before (
main—PluginModernizerTest):After (
fix/verify-plugin-silent-failures—mvn clean test -pl plugin-modernizer-core):Submitter checklist