Skip to content

(re)activate rewrite capability #4708

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Pankraz76
Copy link

@Pankraz76 Pankraz76 commented Jul 1, 2025

activate unused rewrite checks failOnDryRunResults

assuming its not automated, as automation of automation of cosmetics was recently opinionated, rationally not comprehensible, as result is the same, rejected, assuming its not done ether for the real deal, as build success but rewrite unable to be executed on my system:

PS: Wondering how you have it in place, even tho unused, asquarkus and maven (even escaled this) rejected it, because of recently changed licence foo.


I hereby agree to the terms of the JUnit Contributor License Agreement.


Definition of Done

@Pankraz76 Pankraz76 changed the title apply missing rewrite checks failOnDryRunResults activate unused rewrite checks failOnDryRunResults Jul 1, 2025
@marcphilipp
Copy link
Member

assuming its not automated as automation of the automation of cosmetics was rejected assuming its not done for the real deal ether as build success but rewrite unable to be executed on my system:

I don't understand your sentence structure. Could you please elaborate?

PS: Wondering how you have it in place, even tho unused, asquarkus and maven (even escaled this) rejected it, because of recently changed licence foo.

We have it in place as a migration add from Java 8 to 17 for JUnit 6.0. I'm open to using it for static analysis and detecting issues if that's easy to set up.

@Pankraz76 Pankraz76 requested a review from marcphilipp July 1, 2025 14:48
@marcphilipp
Copy link
Member

Looks like this is indeed blocked until Open Rewrite updates to classgraph-4.8.180 or later.

@Pankraz76
Copy link
Author

Pankraz76 commented Jul 1, 2025

yes, but this seems to be an random impl. detail thats going to change again, like expected.

Could potentially fix by downgrade or just wait some time, as issue seems simple and closed soon.

This is more an strategic item wether or not to leverage to tool or kind of get rid or it, as the current benefit seems questionable.

@marcphilipp
Copy link
Member

Looks like this is indeed blocked until Open Rewrite updates to classgraph-4.8.180 or later.

I double-checked and it's already using that version.

This is more an strategic item wether or not to leverage to tool or kind of get rid or it, as the current benefit seems questionable.

I'd like to give it a shot to see whether it's helpful. However, for that I'd have to try it out.

Blocked by openrewrite/rewrite#5677

@Pankraz76
Copy link
Author

item: (on main as well...)

Predictive Test Selection: 17 of 17 test classes selected with profile 'Standard' (saving 0s serial time)

> Task :platform-tooling-support-tests:generateOpenTestHtmlReport FAILED
[Fatal Error] open-test-report.xml:309:817: XML document structures must start and end within the same entity.
org.xml.sax.SAXParseException; systemId: file:/Users/vincent.potucek/IdeaProjects/junit-framework/platform-tooling-support-tests/build/test-results/test/junit-2688237983167731461/open-test-report.xml; lineNumber: 309; columnNumber: 817; XML document structures must start and end within the same entity.
        at java.xml/com.sun.org.apache.xerces.internal.parsers.DOMParser.parse(DOMParser.java:262)
        at java.xml/com.sun.org.apache.xerces.internal.jaxp.DocumentBuilderImpl.parse(DocumentBuilderImpl.java:342)
        at java.xml/javax.xml.parsers.DocumentBuilder.parse(DocumentBuilder.java:206)
        at [email protected]/org.opentest4j.reporting.tooling.core.htmlreport.DefaultHtmlReportWriter.parseDom(DefaultHtmlReportWriter.java:269)
        at [email protected]/org.opentest4j.reporting.tooling.core.htmlreport.DefaultHtmlReportWriter.extractData(DefaultHtmlReportWriter.java:115)
        at [email protected]/org.opentest4j.reporting.tooling.core.htmlreport.DefaultHtmlReportWriter.writeHtmlReport(DefaultHtmlReportWriter.java:92)
        at [email protected]/org.opentest4j.reporting.cli.HtmlReportCommand.call(HtmlReportCommand.java:69)
        at [email protected]/org.opentest4j.reporting.cli.HtmlReportCommand.call(HtmlReportCommand.java:39)
        at [email protected]/picocli.CommandLine.executeUserObject(CommandLine.java:2031)
        at [email protected]/picocli.CommandLine.access$1500(CommandLine.java:148)
        at [email protected]/picocli.CommandLine$RunLast.executeUserObjectOfLastSubcommandWithSameParent(CommandLine.java:2469)
        at [email protected]/picocli.CommandLine$RunLast.handle(CommandLine.java:2461)
        at [email protected]/picocli.CommandLine$RunLast.handle(CommandLine.java:2423)
        at [email protected]/picocli.CommandLine$AbstractParseResultHandler.execute(CommandLine.java:2277)
        at [email protected]/picocli.CommandLine$RunLast.execute(CommandLine.java:2425)
        at [email protected]/picocli.CommandLine.execute(CommandLine.java:2174)
        at [email protected]/org.opentest4j.reporting.cli.ReportingCli.main(ReportingCli.java:52)

[Incubating] Problems report is available at: file:///Users/vincent.potucek/IdeaProjects/junit-framework/build/reports/problems/problems-report.html

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':platform-tooling-support-tests:generateOpenTestHtmlReport'.
> Process 'command '/Users/vincent.potucek/.gradle/jdks/bellsoft-24-aarch64-os_x.2/bin/java'' finished with non-zero exit value 1

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Get more help at https://help.gradle.org.

Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

For more on this, please refer to https://docs.gradle.org/8.14.2/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.

BUILD FAILED in 1m 47s
433 actionable tasks: 268 executed, 7 from cache, 158 up-to-date
➜  junit-framework git:(override) 

@Pankraz76 Pankraz76 force-pushed the override branch 3 times, most recently from e842958 to 035df98 Compare July 1, 2025 17:13
@Pankraz76 Pankraz76 requested a review from marcphilipp July 1, 2025 17:13
@Pankraz76
Copy link
Author

I don't understand your sentence structure. Could you please elaborate?

yes sorry.

As we dont use the auto apply on spot assuming its not done for rewrite ether.

Nice to see that you (at least) consider activating the failOnDryRunResults.

This shows real discipline and consequence.

@Pankraz76
Copy link
Author

Pankraz76 commented Jul 1, 2025

is it possible to have hybrid version of auto fix on dedicated local env value. As now it would be 2 goals to apply or extracting spot apply and rewrite apply into one profile, not to improve config burden on contributor, but giving conventional approach for this new convention that will be imposed.

this is silent approach making both parties happy.

@Pankraz76 Pankraz76 force-pushed the override branch 3 times, most recently from 0637431 to aba289d Compare July 1, 2025 19:30
Comment on lines 289 to 292
tasks.named("rewriteDryRun") {
dependsOn("compileJava")
if (valueOf(getenv("rewriteRun")) && getenv("CI") == null) {
dependsOn("rewriteRun")
Copy link
Contributor

Choose a reason for hiding this comment

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

If a dry run depends on a rewriteRun that might end up performing the slow step of building up a model twice. I think it'd be better to switch between a dry run and and regular run (making code changes) based on whether we're in CI.

@Pankraz76 Pankraz76 changed the title activate unused rewrite checks failOnDryRunResults (re)activate rewrite capability Jul 3, 2025
@Pankraz76
Copy link
Author

add thread to impose open feedback.

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

Successfully merging this pull request may close these issues.

3 participants