From 37cff59cd452dec679ed979750925e0d4bfccfa5 Mon Sep 17 00:00:00 2001 From: shartte Date: Fri, 7 Jun 2024 22:00:42 +0200 Subject: [PATCH] Fix JUnit program argument file format (#12) --- .../moddevgradle/internal/ModDevPlugin.java | 63 +++++++++++-------- .../moddevgradle/internal/PrepareRun.java | 1 + .../internal/PrepareRunOrTest.java | 48 ++++++++++++-- .../moddevgradle/internal/PrepareTest.java | 1 + 4 files changed, 81 insertions(+), 32 deletions(-) diff --git a/src/main/java/net/neoforged/moddevgradle/internal/ModDevPlugin.java b/src/main/java/net/neoforged/moddevgradle/internal/ModDevPlugin.java index 5fbd2a92..045d3158 100644 --- a/src/main/java/net/neoforged/moddevgradle/internal/ModDevPlugin.java +++ b/src/main/java/net/neoforged/moddevgradle/internal/ModDevPlugin.java @@ -72,6 +72,12 @@ public class ModDevPlugin implements Plugin { private static final String TASK_GROUP = "mod development"; + /** + * Name of the configuration in which we place our generated artifacts for use in the runtime classpath, + * without having them leak to dependents. + */ + private static final String CONFIGURATION_GENERATED_ARTIFACTS = "neoForgeGeneratedArtifacts"; + private Runnable configureTesting = null; @Override @@ -192,7 +198,7 @@ public void apply(Project project) { minecraftClassesArtifact = createArtifacts.map(task -> project.files(task.getCompiledArtifact())); } - var localRuntime = configurations.create("neoForgeGeneratedArtifacts", config -> { + var localRuntime = configurations.create(CONFIGURATION_GENERATED_ARTIFACTS, config -> { config.setDescription("Minecraft artifacts that were generated locally by NFRT"); config.setCanBeResolved(false); config.setCanBeConsumed(false); @@ -515,8 +521,8 @@ private void setupTesting(Project project, }); }); - var localRuntime = configurations.getByName("neoForgeGeneratedArtifacts"); var testLocalRuntime = configurations.create("neoForgeTestFixtures", config -> { + config.setDescription("Additional JUnit helpers provided by NeoForge"); config.setCanBeResolved(false); config.setCanBeConsumed(false); config.withDependencies(dependencies -> { @@ -530,11 +536,11 @@ private void setupTesting(Project project, }); configurations.named("testRuntimeClasspath", files -> { - files.extendsFrom(localRuntime); + files.extendsFrom(configurations.getByName(CONFIGURATION_GENERATED_ARTIFACTS)); files.extendsFrom(testLocalRuntime); }); - var legacyClasspathConfiguration = configurations.create("fmljunitLibraries", spec -> { + var legacyClasspathConfiguration = configurations.create("neoForgeTestLibraries", spec -> { spec.setCanBeResolved(true); spec.setCanBeConsumed(false); spec.attributes(attributes -> { @@ -546,55 +552,61 @@ private void setupTesting(Project project, }); }); - var writeLcpTask = tasks.register("writeFmlJunitClasspath", WriteLegacyClasspath.class, writeLcp -> { - writeLcp.getLegacyClasspathFile().convention(modDevDir.map(dir -> dir.file("fmljunitrunVmArgsLegacyClasspath.txt"))); + // Place files for junit runtime in a subdirectory to avoid conflicting with other runs + var runArgsDir = modDevDir.map(dir -> dir.dir("junit")); + + var writeLcpTask = tasks.register("writeNeoForgeTestClasspath", WriteLegacyClasspath.class, writeLcp -> { + writeLcp.getLegacyClasspathFile().convention(runArgsDir.map(dir -> dir.file("legacyClasspath.txt"))); writeLcp.getEntries().from(legacyClasspathConfiguration); writeLcp.getEntries().from(createArtifacts.get().getResourcesArtifact()); }); - var testVmArgsFile = modDevDir.map(dir -> dir.file("fmljunitrunVmArgs.txt")); - var fmlJunitArgsFile = modDevDir.map(dir -> dir.file("fmljunitrunProgramArgs.txt")); - var fmlJunitLog4jConfig = modDevDir.map(dir -> dir.file("fmljunitlog4j2.xml")); - var prepareRunTask = tasks.register("prepareFmlJunitFiles", PrepareTest.class, task -> { + var vmArgsFile = runArgsDir.map(dir -> dir.file("vmArgs.txt")); + var programArgsFile = runArgsDir.map(dir -> dir.file("programArgs.txt")); + var log4j2ConfigFile = runArgsDir.map(dir -> dir.file("log4j2.xml")); + var prepareTask = tasks.register("prepareNeoForgeTestFiles", PrepareTest.class, task -> { task.getGameDirectory().set(unitTest.getGameDirectory()); - task.getVmArgsFile().set(testVmArgsFile); - task.getProgramArgsFile().set(fmlJunitArgsFile); - task.getLog4jConfigFile().set(fmlJunitLog4jConfig); + task.getVmArgsFile().set(vmArgsFile); + task.getProgramArgsFile().set(programArgsFile); + task.getLog4jConfigFile().set(log4j2ConfigFile); task.getNeoForgeModDevConfig().from(userDevConfigOnly); task.getModules().from(neoForgeModDevModules); task.getLegacyClasspathFile().set(writeLcpTask.get().getLegacyClasspathFile()); task.getAssetProperties().set(downloadAssets.flatMap(DownloadAssetsTask::getAssetPropertiesFile)); task.getGameLogLevel().set(Level.INFO); }); - ideSyncTask.configure(task -> task.dependsOn(prepareRunTask)); + + // Ensure the test files are written on sync so that users who use IDE-only tests can run them + ideSyncTask.configure(task -> task.dependsOn(prepareTask)); var testTask = tasks.named("test", Test.class, task -> { - task.dependsOn(prepareRunTask); + task.dependsOn(prepareTask); // The FML JUnit plugin uses this system property to read a // file containing the program arguments needed to launch - task.systemProperty("fml.junit.argsfile", fmlJunitArgsFile.get().getAsFile().getAbsolutePath()); - task.jvmArgs(RunUtils.escapeJvmArg(RunUtils.getArgFileParameter(testVmArgsFile.get()))); + task.systemProperty("fml.junit.argsfile", programArgsFile.get().getAsFile().getAbsolutePath()); + task.jvmArgs(RunUtils.escapeJvmArg(RunUtils.getArgFileParameter(vmArgsFile.get()))); var modFoldersProvider = RunUtils.getGradleModFoldersProvider(project, project.provider(extension::getMods), true); task.getJvmArgumentProviders().add(modFoldersProvider); }); - // Test tasks don't have a provider-based property for working directory, so we need to afterEvaluate it. project.afterEvaluate(p -> { - testTask.configure(task -> { - task.setWorkingDir(unitTest.getGameDirectory()); - }); + // Test tasks don't have a provider-based property for working directory, so we need to afterEvaluate it. + testTask.configure(task -> task.setWorkingDir(unitTest.getGameDirectory())); - // Configure IntelliJ default JUnit parameters, which is used when the user configures IJ to run tests natively + // Configure IntelliJ default JUnit parameters, which are used when the user configures IJ to run tests natively var intelliJRunConfigurations = getIntelliJRunConfigurations(p); if (intelliJRunConfigurations != null) { var outputDirectory = RunUtils.getIntellijOutputDirectory(p); intelliJRunConfigurations.defaults(JUnit.class, jUnitDefaults -> { + jUnitDefaults.setWorkingDirectory(unitTest.getGameDirectory().get().getAsFile().getAbsolutePath()); jUnitDefaults.setVmParameters( - RunUtils.escapeJvmArg("-Dfml.junit.argsfile=" + fmlJunitArgsFile.get().getAsFile().getAbsolutePath()) + // The FML JUnit plugin uses this system property to read a + // file containing the program arguments needed to launch + RunUtils.escapeJvmArg("-Dfml.junit.argsfile=" + programArgsFile.get().getAsFile().getAbsolutePath()) + " " - + RunUtils.escapeJvmArg(RunUtils.getArgFileParameter(testVmArgsFile.get())) + + RunUtils.escapeJvmArg(RunUtils.getArgFileParameter(vmArgsFile.get())) + " " + RunUtils.escapeJvmArg(RunUtils.getIdeaModFoldersProvider(p, outputDirectory, unitTest.getTestedMod().map(Set::of), true).getArgument()) ); @@ -604,9 +616,6 @@ private void setupTesting(Project project, } private static void setupJarJar(Project project) { - //var jarJar = project.getExtensions().create(JarJar.class, "jarJar", JarJarExtension.class); - //((JarJarExtension) jarJar).createTaskAndConfiguration(); - SourceSetContainer sourceSets = ExtensionUtils.getExtension(project, "sourceSets", SourceSetContainer.class); sourceSets.configureEach(sourceSet -> { final Configuration configuration = project.getConfigurations().create(sourceSet.getTaskName(null, "jarJar")); diff --git a/src/main/java/net/neoforged/moddevgradle/internal/PrepareRun.java b/src/main/java/net/neoforged/moddevgradle/internal/PrepareRun.java index 82fd2c1d..7710a371 100644 --- a/src/main/java/net/neoforged/moddevgradle/internal/PrepareRun.java +++ b/src/main/java/net/neoforged/moddevgradle/internal/PrepareRun.java @@ -18,6 +18,7 @@ abstract class PrepareRun extends PrepareRunOrTest { @Inject public PrepareRun() { + super(ProgramArgsFormat.JVM_ARGFILE); } @Override diff --git a/src/main/java/net/neoforged/moddevgradle/internal/PrepareRunOrTest.java b/src/main/java/net/neoforged/moddevgradle/internal/PrepareRunOrTest.java index 345890c7..ddd3ba03 100644 --- a/src/main/java/net/neoforged/moddevgradle/internal/PrepareRunOrTest.java +++ b/src/main/java/net/neoforged/moddevgradle/internal/PrepareRunOrTest.java @@ -21,7 +21,6 @@ import org.jetbrains.annotations.Nullable; import org.slf4j.event.Level; -import javax.inject.Inject; import java.io.File; import java.io.IOException; import java.nio.file.Files; @@ -79,8 +78,10 @@ abstract class PrepareRunOrTest extends DefaultTask { @Input public abstract Property getGameLogLevel(); - @Inject - public PrepareRunOrTest() { + private final ProgramArgsFormat programArgsFormat; + + protected PrepareRunOrTest(ProgramArgsFormat programArgsFormat) { + this.programArgsFormat = programArgsFormat; } protected abstract UserDevRunType resolveRunType(UserDevConfig userDevConfig); @@ -171,12 +172,33 @@ private void writeProgramArguments(UserDevRunType runConfig) throws IOException case "{assets_root}" -> arg = Objects.requireNonNull(assetProperties.assetsRoot(), "assets_root"); case "{asset_index}" -> arg = Objects.requireNonNull(assetProperties.assetIndex(), "asset_index"); } - lines.add(RunUtils.escapeJvmArg(arg)); + + // FML JUnit simply expects one line per argument + if (programArgsFormat == ProgramArgsFormat.FML_JUNIT) { + lines.add(arg); + } else { + lines.add(RunUtils.escapeJvmArg(arg)); + } } lines.add(""); lines.add("# User Supplied Program Arguments"); - lines.addAll(getProgramArguments().get()); + for (var arg : getProgramArguments().get()) { + // FML JUnit simply expects one line per argument + if (programArgsFormat == ProgramArgsFormat.FML_JUNIT) { + lines.add(arg); + } else { + lines.add(RunUtils.escapeJvmArg(arg)); + } + } + + // For FML JUnit, we need to drop comments + empty lines + if (programArgsFormat == ProgramArgsFormat.FML_JUNIT) { + lines.removeIf(line -> { + line = line.strip(); + return line.isEmpty() || line.startsWith("#"); + }); + } FileUtils.writeLinesSafe(getProgramArgsFile().get().getAsFile().toPath(), lines); } @@ -184,4 +206,20 @@ private void writeProgramArguments(UserDevRunType runConfig) throws IOException private static void addSystemProp(String name, String value, List lines) { lines.add(RunUtils.escapeJvmArg("-D" + name + "=" + value)); } + + /** + * Declares the format of the program arguments file being written. + */ + protected enum ProgramArgsFormat { + /** + * Format as used by JVM @-files + */ + JVM_ARGFILE, + /** + * Format as used by FML JUnit, which expects + * a file to be passed in the system property "fml.junit.argsfile", containing one program argument per line + * without consideration for escaping. + */ + FML_JUNIT + } } diff --git a/src/main/java/net/neoforged/moddevgradle/internal/PrepareTest.java b/src/main/java/net/neoforged/moddevgradle/internal/PrepareTest.java index d6b1c535..a9907ea3 100644 --- a/src/main/java/net/neoforged/moddevgradle/internal/PrepareTest.java +++ b/src/main/java/net/neoforged/moddevgradle/internal/PrepareTest.java @@ -8,6 +8,7 @@ abstract class PrepareTest extends PrepareRunOrTest { @Inject public PrepareTest() { + super(ProgramArgsFormat.FML_JUNIT); } @Override