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.
This commit is contained in:
Stephan Schroevers
2024-11-18 06:32:51 +01:00
committed by GitHub
parent 37cbee0f0a
commit fff368c80a
5 changed files with 1214 additions and 407 deletions

View File

@@ -93,6 +93,12 @@ final class FileRules {
/**
* Prefer {@link Files#createTempFile(String, String, FileAttribute[])} over alternatives that
* create files with more liberal permissions.
*
* <p>Note that {@link File#createTempFile} treats the given prefix as a path, and ignores all but
* its file name. That is, the actual prefix used is derived from all characters following the
* final file separator (if any). This is not the case with {@link Files#createTempFile}, which
* will instead throw an {@link IllegalArgumentException} if the prefix contains any file
* separators.
*/
static final class FilesCreateTempFileToFile {
@BeforeTemplate
@@ -117,6 +123,12 @@ final class FileRules {
/**
* Prefer {@link Files#createTempFile(Path, String, String, FileAttribute[])} over alternatives
* that create files with more liberal permissions.
*
* <p>Note that {@link File#createTempFile} treats the given prefix as a path, and ignores all but
* its file name. That is, the actual prefix used is derived from all characters following the
* final file separator (if any). This is not the case with {@link Files#createTempFile}, which
* will instead throw an {@link IllegalArgumentException} if the prefix contains any file
* separators.
*/
static final class FilesCreateTempFileInCustomDirectoryToFile {
@BeforeTemplate

File diff suppressed because it is too large Load Diff

View File

@@ -134,6 +134,26 @@
public class TreeWalkerTest extends AbstractModuleTestSupport {
@TempDir
--- 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);
}
--- a/src/test/java/com/puppycrawl/tools/checkstyle/utils/CheckUtilTest.java
+++ b/src/test/java/com/puppycrawl/tools/checkstyle/utils/CheckUtilTest.java
@@ -47,6 +47,8 @@ import com.puppycrawl.tools.checkstyle.checks.coding.NestedIfDepthCheck;

View File

@@ -7,7 +7,7 @@ project='checkstyle'
repository='https://github.com/checkstyle/checkstyle.git'
revision='checkstyle-10.14.0'
# XXX: Configure Renovate to manage the AssertJ version declared here.
additional_build_flags='-Dassertj.version=3.24.2'
additional_build_flags='-Perror-prone-compile,error-prone-test-compile -Dassertj.version=3.24.2 -Dmaven.compiler.failOnError=true'
additional_source_directories='${project.basedir}${file.separator}src${file.separator}it${file.separator}java,${project.basedir}${file.separator}src${file.separator}xdocs-examples${file.separator}java'
patch_error_prone_flags=''
validation_error_prone_flags=''

View File

@@ -52,7 +52,6 @@ case "$(uname -s)" in
esac
shared_build_flags="
-Perror-prone-compile,error-prone-test-compile
-Derror-prone.version=$(
mvn -f "${error_prone_support_root}" help:evaluate -Dexpression=version.error-prone -q -DforceStdout
)
@@ -141,10 +140,13 @@ diff_base="$(git rev-parse HEAD)"
function apply_patch() {
local extra_build_args="${1}"
mvn ${shared_build_flags} ${extra_build_args} \
package "${format_goal}" \
-Derror-prone.configuration-args="${error_prone_patch_flags}" \
-DskipTests
(
set -x \
&& mvn ${shared_build_flags} ${extra_build_args} \
package "${format_goal}" \
-Derror-prone.configuration-args="${error_prone_patch_flags}" \
-DskipTests
)
if ! git diff --exit-code; then
git commit -m 'minor: Apply patches' .
@@ -168,12 +170,13 @@ apply_patch ''
# By also running the tests, we validate that the (majority of) applied changes
# are behavior preserving.
validation_build_log="${report_directory}/${test_name}-validation-build-log.txt"
mvn ${shared_build_flags} \
clean package \
-Derror-prone.configuration-args="${error_prone_validation_flags}" \
${validation_build_flags} \
| tee "${validation_build_log}" \
|| failure=1
(
set -x \
&& mvn ${shared_build_flags} \
clean package \
-Derror-prone.configuration-args="${error_prone_validation_flags}" \
${validation_build_flags}
) | tee "${validation_build_log}" || failure=1
# Collect the applied changes.
expected_changes="${integration_test_root}/${test_name}-expected-changes.patch"