From 6bac8d7e8517a14ebf94c229f93429285a4443cf Mon Sep 17 00:00:00 2001 From: Nikan Radan Date: Fri, 31 Oct 2025 19:24:27 -0700 Subject: [PATCH] Test Clean Up These are just some simple test fixes, mostly caught by IntelliJ's static analysis and SonarQube's static analysis. Below are the changes: 1. Corrected order of parameters for `assertEquals`. `expected` should be the first parameter with `actual` being the second parameter. 2. Remove unnecessary `throws IOException` in `McVersionLookupTest`. As a note, all parameters and the return for `visitFile` should be marked NotNull (or NonNull if you migrate to JSpecify). An interface may state the throw in the function signature but in Java, if overriding a method, you cannot declare new or broader checked exceptions but you can declare fewer, and if your implementation doesn't throw, then you shouldn't include it in the override's signature (which is why IDEs like IntelliJ will tell you to remove it). 3. Statement lambdas were replaced with expression lambdas where appropriate. 4. Some tests used `Assertions.assertEquals` while others just directly imported `assertEquals`. I changed all instances of `Assertions.assertEquals` to `assertEquals` to keep things consistent. 5. In `V1ModJsonParsingTests`, the field `private static Path testLocation` was converted to a local variable. It was never used outside of `setupPaths`. 6. An `assertTrue` check was correctly changed to `assertInstanceOf` in `V1ModJsonParsingTests`. 7. Instances of explicit type arguments that were redundant were simplified to `<>`. This doesn't change any functionality, all tests run the same. This only corrects some semantics (like incorrect `assertEquals` order) and cleans up code. --- .../minecraft/test/junit/JunitTest.java | 2 +- .../fabricmc/test/McVersionLookupTest.java | 2 +- ...ersionNormalizationAntiRegressionTest.java | 24 ++++++++++--------- .../test/VersionNormalizationTest.java | 11 +++++---- .../impl/discovery/GetNonFabricModsTest.java | 3 +-- .../fabricmc/test/ArgumentParsingTests.java | 15 ++++++------ .../fabricmc/test/LogNonFabricModsTest.java | 2 +- .../fabricmc/test/V1ModJsonParsingTests.java | 24 ++++++++----------- 8 files changed, 41 insertions(+), 42 deletions(-) diff --git a/minecraft/minecraft-test/src/test/java/net/fabricmc/minecraft/test/junit/JunitTest.java b/minecraft/minecraft-test/src/test/java/net/fabricmc/minecraft/test/junit/JunitTest.java index 42de7a37d..3fdafe5ae 100644 --- a/minecraft/minecraft-test/src/test/java/net/fabricmc/minecraft/test/junit/JunitTest.java +++ b/minecraft/minecraft-test/src/test/java/net/fabricmc/minecraft/test/junit/JunitTest.java @@ -44,7 +44,7 @@ public static void setup() { @Test public void testItems() { Identifier id = Registries.ITEM.getId(Items.DIAMOND); - assertEquals(id.toString(), "minecraft:diamond"); + assertEquals("minecraft:diamond", id.toString()); System.out.println(id); } diff --git a/minecraft/src/test/java/net/fabricmc/test/McVersionLookupTest.java b/minecraft/src/test/java/net/fabricmc/test/McVersionLookupTest.java index 8c6f2b839..7c26f4dbf 100644 --- a/minecraft/src/test/java/net/fabricmc/test/McVersionLookupTest.java +++ b/minecraft/src/test/java/net/fabricmc/test/McVersionLookupTest.java @@ -45,7 +45,7 @@ public static void main(String[] args) throws IOException { if (Files.isDirectory(path)) { Files.walkFileTree(path, new SimpleFileVisitor() { @Override - public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) { if (file.getFileName().toString().endsWith(".jar")) { check(file, path.relativize(file).toString(), invalid); } diff --git a/minecraft/src/test/java/net/fabricmc/test/VersionNormalizationAntiRegressionTest.java b/minecraft/src/test/java/net/fabricmc/test/VersionNormalizationAntiRegressionTest.java index cdf8cb1ed..7785cf0e3 100644 --- a/minecraft/src/test/java/net/fabricmc/test/VersionNormalizationAntiRegressionTest.java +++ b/minecraft/src/test/java/net/fabricmc/test/VersionNormalizationAntiRegressionTest.java @@ -75,8 +75,9 @@ public void setup() { Type listType = new TypeToken>() { }.getType(); - try (Reader reader = new InputStreamReader(VersionNormalizationAntiRegressionTest.class.getClassLoader() - .getResourceAsStream(MINECRAFT_VERSIONS_RESOURCE))) { + try (Reader reader = new InputStreamReader(Objects + .requireNonNull(VersionNormalizationAntiRegressionTest.class.getClassLoader() + .getResourceAsStream(MINECRAFT_VERSIONS_RESOURCE)))) { expectedResults = gson.fromJson(reader, listType); } catch (IOException e) { throw new RuntimeException("Failed to read in existing versions from json", e); @@ -174,8 +175,9 @@ public void confirmAllUnique() throws VersionParsingException { private Set> getRereleasedVersions() { Set> vs = new HashSet<>(); - try (Reader reader = new InputStreamReader(VersionNormalizationAntiRegressionTest.class.getClassLoader() - .getResourceAsStream("duplicate_versions.json"))) { + try (Reader reader = new InputStreamReader(Objects + .requireNonNull(VersionNormalizationAntiRegressionTest.class.getClassLoader() + .getResourceAsStream("duplicate_versions.json")))) { JsonParser parser = new JsonParser(); for (JsonElement grpElem : parser.parse(reader).getAsJsonObject().getAsJsonArray("duplicates")) { @@ -254,9 +256,8 @@ private static List getMojangData() throws MalformedURLExcepti try (Reader in = new InputStreamReader(url.openStream())) { PistonMetaV2 pistonMetaV2 = gson.fromJson(in, PistonMetaV2.class); - pistonMetaV2.versions.forEach(v -> { - minecraftVersions.add(new MinecraftVersion(v.id, v.releaseTime.toInstant())); - }); + pistonMetaV2.versions.forEach(v -> + minecraftVersions.add(new MinecraftVersion(v.id, v.releaseTime.toInstant()))); } catch (IOException e) { throw new RuntimeException("Failed to read PistonV2 meta", e); } @@ -279,9 +280,8 @@ private static List getFabricMirror() throws MalformedURLExcep try (Reader in = new InputStreamReader(url.openStream())) { FabricMeta fabricMeta = gson.fromJson(in, FabricMeta.class); - fabricMeta.versions.forEach(v -> { - minecraftVersions.add(new MinecraftVersion(v.id, v.releaseTime.toInstant())); - }); + fabricMeta.versions.forEach(v -> + minecraftVersions.add(new MinecraftVersion(v.id, v.releaseTime.toInstant()))); } catch (IOException e) { throw new RuntimeException("Failed to read PistonV2 meta", e); } @@ -351,7 +351,7 @@ private static List getVersionsFromOmniArchive(String urlS) th } private static List parseCsvLine(String line) { - // Let's just do the simple parsing for this, it isn't run often anyways + // Let's just do the simple parsing for this, it isn't run often anyway return Arrays.asList(line.split(",")); } @@ -359,6 +359,7 @@ private static List parseCsvLine(String line) { * Metadata. */ private static class PistonMetaV2 { + @SuppressWarnings("MismatchedQueryAndUpdateOfCollection") List versions; static class Version { @@ -372,6 +373,7 @@ static class Version { * Metadata. */ private static class FabricMeta { + @SuppressWarnings("MismatchedQueryAndUpdateOfCollection") List versions; static class Version { diff --git a/minecraft/src/test/java/net/fabricmc/test/VersionNormalizationTest.java b/minecraft/src/test/java/net/fabricmc/test/VersionNormalizationTest.java index d3c3b2565..a93dd6ae2 100644 --- a/minecraft/src/test/java/net/fabricmc/test/VersionNormalizationTest.java +++ b/minecraft/src/test/java/net/fabricmc/test/VersionNormalizationTest.java @@ -16,12 +16,13 @@ package net.fabricmc.test; +import static org.junit.jupiter.api.Assertions.assertEquals; + import java.util.ArrayList; import java.util.Arrays; import java.util.List; import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -241,7 +242,7 @@ public void tearDown() { public void testGetRelease() { for (MinecraftVersion result : expectedResults) { for (NormalizedVersion entry : result.entries) { - Assertions.assertEquals(entry.release, McVersionLookup.getRelease(entry.version), "getRelease(" + entry.version + ")"); + assertEquals(entry.release, McVersionLookup.getRelease(entry.version), "getRelease(" + entry.version + ")"); } } } @@ -250,7 +251,7 @@ public void testGetRelease() { public void testNormalizeVersion() { for (MinecraftVersion result : expectedResults) { for (NormalizedVersion entry : result.entries) { - Assertions.assertEquals(entry.normalizedVersion, McVersionLookup.normalizeVersion(entry.version, entry.release), "normalizeVersion(" + entry.version + ", " + entry.release + ")"); + assertEquals(entry.normalizedVersion, McVersionLookup.normalizeVersion(entry.version, entry.release), "normalizeVersion(" + entry.version + ", " + entry.release + ")"); } } } @@ -270,9 +271,9 @@ public void testVersionComparison() throws Exception { Version v2 = result2.entries.get(l).semver(); if (i == k) { - Assertions.assertEquals(true, v1.compareTo(v2) == 0, v1.toString() + " == " + v2.toString()); + assertEquals(0, v1.compareTo(v2), v1 + " == " + v2); } else { - Assertions.assertEquals(i > k, v1.compareTo(v2) > 0, v1.toString() + " > " + v2.toString()); + assertEquals(i > k, v1.compareTo(v2) > 0, v1 + " > " + v2); } } } diff --git a/src/test/java/net/fabricmc/loader/impl/discovery/GetNonFabricModsTest.java b/src/test/java/net/fabricmc/loader/impl/discovery/GetNonFabricModsTest.java index 636440eaa..a42d8cc88 100644 --- a/src/test/java/net/fabricmc/loader/impl/discovery/GetNonFabricModsTest.java +++ b/src/test/java/net/fabricmc/loader/impl/discovery/GetNonFabricModsTest.java @@ -24,7 +24,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; -import java.util.Set; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; @@ -77,7 +76,7 @@ public void tearDown() { */ @Test public void testGetNonFabricMods() throws ModResolutionException { - List acceptedMods = discoverer.discoverMods(loader, new HashMap>()); + List acceptedMods = discoverer.discoverMods(loader, new HashMap<>()); boolean foundDummyFabricMod = false; diff --git a/src/test/java/net/fabricmc/test/ArgumentParsingTests.java b/src/test/java/net/fabricmc/test/ArgumentParsingTests.java index 22d8e6168..72a199ae7 100644 --- a/src/test/java/net/fabricmc/test/ArgumentParsingTests.java +++ b/src/test/java/net/fabricmc/test/ArgumentParsingTests.java @@ -16,7 +16,8 @@ package net.fabricmc.test; -import org.junit.jupiter.api.Assertions; +import static org.junit.jupiter.api.Assertions.assertEquals; + import org.junit.jupiter.api.Test; import net.fabricmc.loader.impl.util.Arguments; @@ -28,9 +29,9 @@ public void parseNormal() { arguments.parse(new String[]{"--clientId", "123", "--xuid", "abc", "--versionType", "release"}); arguments.put("versionType", "Fabric"); - Assertions.assertEquals(arguments.keys().size(), 3); - Assertions.assertEquals(arguments.get("xuid"), "abc"); - Assertions.assertEquals(arguments.get("versionType"), "Fabric"); + assertEquals(3, arguments.keys().size()); + assertEquals("abc", arguments.get("xuid")); + assertEquals("Fabric", arguments.get("versionType")); } @Test @@ -39,8 +40,8 @@ public void parseMissing() { arguments.parse(new String[]{"--clientId", "123", "--xuid", "--versionType", "release"}); arguments.put("versionType", "Fabric"); - Assertions.assertEquals(arguments.keys().size(), 3); - Assertions.assertEquals(arguments.get("xuid"), ""); - Assertions.assertEquals(arguments.get("versionType"), "Fabric"); + assertEquals(3, arguments.keys().size()); + assertEquals("", arguments.get("xuid")); + assertEquals("Fabric", arguments.get("versionType")); } } diff --git a/src/test/java/net/fabricmc/test/LogNonFabricModsTest.java b/src/test/java/net/fabricmc/test/LogNonFabricModsTest.java index d168200ba..cc78c0517 100644 --- a/src/test/java/net/fabricmc/test/LogNonFabricModsTest.java +++ b/src/test/java/net/fabricmc/test/LogNonFabricModsTest.java @@ -53,7 +53,7 @@ public void setUp() { */ @Test public void testLogNonFabricMods() { - List nonFabricMods = new ArrayList(); + List nonFabricMods = new ArrayList<>(); nonFabricMods.add(Paths.get("mods/non_fabric_mod1.jar")); nonFabricMods.add(Paths.get("mods/non_fabric_mod2.jar")); nonFabricMods.add(Paths.get("mods/non_fabric_mod3.jar")); diff --git a/src/test/java/net/fabricmc/test/V1ModJsonParsingTests.java b/src/test/java/net/fabricmc/test/V1ModJsonParsingTests.java index 67af61777..c1267a9d5 100644 --- a/src/test/java/net/fabricmc/test/V1ModJsonParsingTests.java +++ b/src/test/java/net/fabricmc/test/V1ModJsonParsingTests.java @@ -16,11 +16,6 @@ package net.fabricmc.test; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; - import java.io.File; import java.io.IOException; import java.io.InputStream; @@ -43,14 +38,15 @@ import net.fabricmc.loader.impl.metadata.ParseMetadataException; import net.fabricmc.loader.impl.metadata.VersionOverrides; +import static org.junit.jupiter.api.Assertions.*; + final class V1ModJsonParsingTests { - private static Path testLocation; private static Path specPath; private static Path errorPath; @BeforeAll public static void setupPaths() { - testLocation = new File(System.getProperty("user.dir")) + Path testLocation = new File(System.getProperty("user.dir")) .toPath() .resolve("src") .resolve("test") @@ -142,7 +138,7 @@ private void validateRequiredValues(LoaderModMetadata metadata) { final String friendlyString = metadata.getVersion().getFriendlyString(); assertEquals("1.0.0-SNAPSHOT", friendlyString, String.format("Version \"%s\" was found, expected \"1.0.0-SNAPSHOT\"", friendlyString)); - assertTrue(metadata.getVersion() instanceof SemanticVersion, "Parsed version was not a semantic version, expected a semantic version"); + assertInstanceOf(SemanticVersion.class, metadata.getVersion(), "Parsed version was not a semantic version, expected a semantic version"); } @Test @@ -202,16 +198,16 @@ private void validateIconPath(LoaderModMetadata metadata, int preferredSize, int @Test public void verifyMissingVersionFails() { // Missing version should throw an exception - assertThrows(ParseMetadataException.MissingField.class, () -> { - parseMetadata(errorPath.resolve("missing_version.json")); - }, "Missing version exception was not caught"); + assertThrows(ParseMetadataException.MissingField.class, () -> + parseMetadata(errorPath.resolve("missing_version.json")), + "Missing version exception was not caught"); } @Test public void validateDuplicateSchemaVersionMismatchFails() { - assertThrows(ParseMetadataException.class, () -> { - parseMetadata(errorPath.resolve("missing_version.json")); - }, "Parser did not fail when the duplicate \"schemaVersion\" mismatches"); + assertThrows(ParseMetadataException.class, () -> + parseMetadata(errorPath.resolve("missing_version.json")), + "Parser did not fail when the duplicate \"schemaVersion\" mismatches"); } /*