Skip to content
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

Update Checkstyle integration test #1424

Merged
merged 1 commit into from
Nov 18, 2024

Conversation

Stephan202
Copy link
Member

@Stephan202 Stephan202 commented Nov 17, 2024

Suggested commit message:

Update Checkstyle integration test (#1424)

Summary of changes:
- Move Checkstyle-specific build parameters out of
  `run-integration-test.sh`.
- Build with `-Dmaven.compiler.failOnError=true` to compensate for
  `<failOnError>false</failOnError>` configured by the enabled Maven
  profiles.
- Tweak `XdocGenerator.java` logic prior to integration test execution,
  to work around a subtle semantic difference introduced by the
  `FilesCreateTempFileToFile` Refaster rule.
- Document this difference on the relevant Refaster rules.
- To aid debugging, run Maven commands with `set -x`, such that the
  exact command executed is logged.
- Update the patch file containing the expected changes.

Summary of changes:
- Move Checkstyle-specific build parameters out of
  `run-integration-test.sh`.
- Build with `-Dmaven.compiler.failOnError=true` to compensate for
  `<failOnError>false</failOnError>` configured by the enabled Maven
  profiles.
- Tweak `XdocGenerator.java` logic prior to integration test execution,
  to work around a subtle semantic difference introduced by the
  `FilesCreateTempFileToFile` Refaster rule.
- Document this difference on the relevant Refaster rules.
- To aid debugging, run Maven commands with `set -x`, such that the
  exact command executed is logged.
- Update the patch file containing the expected changes.
@Stephan202 Stephan202 added the chore A task not related to code (build, formatting, process, ...) label Nov 17, 2024
@Stephan202 Stephan202 added this to the 0.20.0 milestone Nov 17, 2024
Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

Comment on lines +137 to +156
--- a/src/test/java/com/puppycrawl/tools/checkstyle/internal/utils/XdocGenerator.java
+++ b/src/test/java/com/puppycrawl/tools/checkstyle/internal/utils/XdocGenerator.java
@@ -55,7 +55,8 @@ public final class XdocGenerator {
for (Path path : templatesFilePaths) {
final String pathToFile = path.toString();
final File inputFile = new File(pathToFile);
- final File tempFile = File.createTempFile(pathToFile.replace(".template", ""), "");
+ final File outputFile = new File(pathToFile.replace(".template", ""));
+ final File tempFile = File.createTempFile(outputFile.getName(), "");
tempFile.deleteOnExit();
final XdocsTemplateSinkFactory sinkFactory = (XdocsTemplateSinkFactory)
plexus.lookup(SinkFactory.ROLE, XDOCS_TEMPLATE_HINT);
@@ -70,7 +71,6 @@ public final class XdocGenerator {
finally {
sink.close();
}
- final File outputFile = new File(pathToFile.replace(".template", ""));
final StandardCopyOption copyOption = StandardCopyOption.REPLACE_EXISTING;
Files.copy(tempFile.toPath(), outputFile.toPath(), copyOption);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

I filed checkstyle/checkstyle#15935 to upstream these changes.

Copy link
Contributor

@mohamedsamehsalah mohamedsamehsalah left a comment

Choose a reason for hiding this comment

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

Summary LGTM 🚀

Copy link
Member

@rickie rickie left a comment

Choose a reason for hiding this comment

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

Amazing work @Stephan202 thanks for diving into this!

@rickie
Copy link
Member

rickie commented Nov 17, 2024

/integration-test

@Stephan202
Copy link
Member Author

/integration-test

The check isn't listed under "All checks have passed", but it was successful. Will merge PR :)

@Stephan202 Stephan202 merged commit fff368c into master Nov 18, 2024
16 checks passed
@Stephan202 Stephan202 deleted the sschroevers/sync-integration-test branch November 18, 2024 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore A task not related to code (build, formatting, process, ...)
Development

Successfully merging this pull request may close these issues.

3 participants