Skip to content

Accept marker-only changes in RewriteTest instead of failing with empty diff#7350

Closed
timtebeek wants to merge 1 commit intomainfrom
fix/skip-marker-only-results
Closed

Accept marker-only changes in RewriteTest instead of failing with empty diff#7350
timtebeek wants to merge 1 commit intomainfrom
fix/skip-marker-only-results

Conversation

@timtebeek
Copy link
Copy Markdown
Member

@timtebeek timtebeek commented Apr 11, 2026

Summary

Context

  • PR Expand star imports in ChangePackage and related recipes when they would create ambiguity #7202 added JavaSourceSetUpdater to ChangeDependencyGroupIdAndArtifactId, which correctly updates classpath markers on Java files when dependency coordinates change. However, this caused downstream test failures in repos that assert on Maven dependency changes alongside Java source files — the Java files produce a Result (because the marker changed the object identity) but have identical printed text, triggering the "empty diff" guard in RewriteTest.

Affected repos: rewrite-hibernate, rewrite-migrate-java, rewrite-dropwizard (all broke on scheduled CI April 11).

What changed

In RewriteTest.java, when a Result has identical before/after text, instead of calling fail() we now:

  1. Remove the result from allResults (so no spurious diff is reported)
  2. Still invoke the afterRecipe consumer with result.getAfter() (preserving marker assertions)
  3. Continue to the next source file

This does not affect:

  • RecipeRunCycle per-cycle change tracking (marker changes still count for recipe scheduling)
  • InMemoryLargeSourceSet.getChangeset() (Results are still generated correctly)
  • Any recipe behavior — only the test framework assertion changes

Test plan

  • ChangeDependencyGroupIdAndArtifactIdTest.updatesJavaSourceSetMarkerOnJavaFiles passes (verifies afterRecipe still receives updated markers)
  • ChangeDependencyGroupIdAndArtifactIdTest.javaSourceSetUnchangedWhenModuleDoesNotHaveDependency passes
  • Full rewrite-test module tests pass
  • Full rewrite-maven module tests pass
  • Downstream rewrite-hibernate MigrateToHibernate61Test.groupIdHibernateOrmRenamed passes on main with this SNAPSHOT

…ty diff

After #7202, `ChangeDependencyGroupIdAndArtifactId` updates `JavaSourceSet`
markers on Java files when dependency coordinates change. These marker-only
changes produce Results with identical before/after text, which
`RewriteTest` previously treated as errors ("An empty diff was generated").

Instead of failing, silently accept Results where the printed text is
unchanged. This allows recipes to update markers (e.g. classpath metadata)
without requiring downstream tests to add explicit `after` text for every
Java source spec.

The `afterRecipe` callback still fires with the updated tree, so tests
that need to verify marker changes (like `updatesJavaSourceSetMarkerOnJavaFiles`)
continue to work.

Fixes #7349
@github-project-automation github-project-automation Bot moved this to In Progress in OpenRewrite Apr 11, 2026
@timtebeek timtebeek marked this pull request as draft April 11, 2026 19:01
@timtebeek timtebeek closed this Apr 11, 2026
@github-project-automation github-project-automation Bot moved this from In Progress to Done in OpenRewrite Apr 11, 2026
@timtebeek
Copy link
Copy Markdown
Member Author

Ok getting ticked off here about the wrong changes being made; THis and the downstream PRs are rubbish; the actual problems are likely to do with

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

ChangeDependencyGroupIdAndArtifactId generates empty diffs on Java files after #7202

1 participant