Skip to content

Commit 72d4280

Browse files
tpwalke2tpwalke2
authored
Merge pull request #155 from tpwalke2/feature/tpwalke2/144-version1loader
#144 Add tests for Version1SignEntryLoader
2 parents cb3eb6b + f7cf81b commit 72d4280

3 files changed

Lines changed: 107 additions & 13 deletions

File tree

‎plans/unit-test-coverage-gaps.md‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -139,15 +139,19 @@ partial coverage, plus a few items that look like test gaps but aren't.
139139
`Version3Converter`, then backs up the original file to `<path>.v2.bak` before returning the converted entries);
140140
and the catch-all fallback returning `null` rather than throwing, for both a JSON syntax error and empty content
141141
(which parses to `null` and then NPEs on `versionedSignFile.version()` — caught by the same generic `catch`).
142-
- **`Version1SignEntryLoader`** (`core/signs/persistence/loaders/Version1SignEntryLoader.java`) — only the
143-
already-namespaced dimension string path is exercised today. The actual legacy-shorthand normalization branch
144-
(`"nether"`/`"end"`/`"overworld"` → canonical identifier — the Low-severity finding about incomplete literal
145-
matching) is explicitly *not* tested, per the comment in `LegacySignFileMigratorTest`, because it reaches into
146-
`net.minecraft.world.level.Level`'s static constants, which need a Minecraft bootstrap. Recommend a small
147-
testability seam first — replace `Level.NETHER.identifier().toString()` etc. with the equivalent literal strings
148-
(`"minecraft:the_nether"`, `"minecraft:the_end"`, `"minecraft:overworld"`) so this branch no longer needs a
149-
bootstrap to test — then add cases for the three recognized literals, an unrecognized legacy string falling through
150-
unchanged (the actual bug), and case-insensitivity.
142+
- **`Version1SignEntryLoader`** (`core/signs/persistence/loaders/Version1SignEntryLoader.java`) — DONE. Required the
143+
planned testability seam: replaced `Level.NETHER.identifier().toString()` / `Level.END...` / `Level.OVERWORLD...`
144+
with the equivalent literal strings (`"minecraft:the_nether"`, `"minecraft:the_end"`, `"minecraft:overworld"`),
145+
removing the class's only Minecraft-type dependency (`net.minecraft.world.level.Level`) so it no longer needs a
146+
game bootstrap to test. `Version1SignEntryLoaderTest` (6 cases) covers: the three recognized legacy shorthand
147+
strings (`"nether"`/`"end"`/`"overworld"`) normalizing to their canonical identifiers; case-insensitivity of that
148+
normalization; an already-namespaced dimension string (e.g. `"minecraft:overworld"`) passing through unchanged;
149+
and `loadSignEntries` backing up the original file to `<path>.v1.bak`.
150+
151+
Also documents the Low-severity finding: an unrecognized legacy dimension string doesn't fall through truly
152+
unchanged — `getNormalizedMapId` unconditionally lowercases its input before the `switch`, even on the `default`
153+
branch, so e.g. `"My_Custom_Dimension"` survives as `"my_custom_dimension"` rather than being preserved as-is or
154+
flagged as unrecognized.
151155

152156
## Not test gaps — flagged for a different disposition
153157

‎src/main/java/com/tpwalke2/bluemapsignmarkers/core/signs/persistence/loaders/Version1SignEntryLoader.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
import org.slf4j.LoggerFactory;
1212

1313
import java.util.Arrays;
14-
import net.minecraft.world.level.Level;
1514

1615
public class Version1SignEntryLoader {
1716
private static final Logger LOGGER = LoggerFactory.getLogger(Constants.MOD_ID);
@@ -50,10 +49,12 @@ private static SignEntryKey withNormalizedMapId(SignEntryKey key) {
5049
private static String getNormalizedMapId(String mapId) {
5150
var result = mapId.toLowerCase();
5251

52+
// Literal values of Level.NETHER/END/OVERWORLD's identifiers, spelled out so this class has no
53+
// Minecraft-type dependency and can be unit tested without a game bootstrap.
5354
return switch (result) {
54-
case NETHER -> Level.NETHER.identifier().toString();
55-
case END -> Level.END.identifier().toString();
56-
case OVERWORLD -> Level.OVERWORLD.identifier().toString();
55+
case NETHER -> "minecraft:the_nether";
56+
case END -> "minecraft:the_end";
57+
case OVERWORLD -> "minecraft:overworld";
5758
default -> result;
5859
};
5960

‎src/test/java/com/tpwalke2/bluemapsignmarkers/core/signs/persistence/loaders/Version1SignEntryLoaderTest.java‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
package com.tpwalke2.bluemapsignmarkers.core.signs.persistence.loaders;
2+
3+
import com.google.gson.Gson;
4+
import com.tpwalke2.bluemapsignmarkers.core.markers.MarkerGroup;
5+
import com.tpwalke2.bluemapsignmarkers.core.markers.MarkerGroupMatchType;
6+
import com.tpwalke2.bluemapsignmarkers.core.markers.MarkerGroupType;
7+
import com.tpwalke2.bluemapsignmarkers.core.signs.SignEntryKey;
8+
import com.tpwalke2.bluemapsignmarkers.core.signs.persistence.models.MarkerTypeV2;
9+
import com.tpwalke2.bluemapsignmarkers.core.signs.persistence.models.SignEntryV2;
10+
import com.tpwalke2.bluemapsignmarkers.core.signs.persistence.models.SignLinesParseResultV2;
11+
import org.junit.jupiter.api.Test;
12+
import org.junit.jupiter.api.io.TempDir;
13+
14+
import java.io.IOException;
15+
import java.nio.charset.StandardCharsets;
16+
import java.nio.file.Files;
17+
import java.nio.file.Path;
18+
19+
import static org.junit.jupiter.api.Assertions.assertEquals;
20+
import static org.junit.jupiter.api.Assertions.assertTrue;
21+
22+
class Version1SignEntryLoaderTest {
23+
24+
private static final Gson GSON = new Gson();
25+
private static final MarkerGroup[] POI_GROUP = {
26+
new MarkerGroup("[poi]", MarkerGroupMatchType.STARTS_WITH, MarkerGroupType.POI,
27+
"[poi]", null, 0, 0, false, 0, 0)
28+
};
29+
30+
private static String load(Path tempDir, String legacyDimension) throws IOException {
31+
var path = tempDir.resolve("signs.json").toString();
32+
var entry = new SignEntryV2(
33+
new SignEntryKey(0, 64, 0, legacyDimension),
34+
"player-1",
35+
new SignLinesParseResultV2(MarkerTypeV2.POI, "Town Hall", "Town Hall"),
36+
new SignLinesParseResultV2(null, "", ""));
37+
var content = GSON.toJson(new SignEntryV2[]{entry});
38+
Files.writeString(Path.of(path), content, StandardCharsets.UTF_8);
39+
40+
var result = Version1SignEntryLoader.loadSignEntries(path, content, POI_GROUP, GSON);
41+
42+
return result[0].key().parentMap();
43+
}
44+
45+
@Test
46+
void normalizesTheThreeRecognizedLegacyShorthandDimensions(@TempDir Path tempDir) throws IOException {
47+
assertEquals("minecraft:the_nether", load(tempDir, "nether"));
48+
assertEquals("minecraft:the_end", load(tempDir, "end"));
49+
assertEquals("minecraft:overworld", load(tempDir, "overworld"));
50+
}
51+
52+
@Test
53+
void legacyShorthandNormalizationIsCaseInsensitive(@TempDir Path tempDir) throws IOException {
54+
assertEquals("minecraft:the_nether", load(tempDir, "NETHER"));
55+
assertEquals("minecraft:the_end", load(tempDir, "End"));
56+
assertEquals("minecraft:overworld", load(tempDir, "OverWorld"));
57+
}
58+
59+
// Documents the Low-severity finding: getNormalizedMapId always lowercases its input, even on the default
60+
// branch, so an unrecognized legacy dimension string doesn't pass through truly unchanged - it survives with
61+
// its casing silently altered, rather than being left exactly as-is or flagged as unrecognized.
62+
@Test
63+
void anUnrecognizedLegacyDimensionStringFallsThroughLowercasedButOtherwiseUnchanged(@TempDir Path tempDir) throws IOException {
64+
assertEquals("my_custom_dimension", load(tempDir, "My_Custom_Dimension"));
65+
}
66+
67+
@Test
68+
void anAlreadyNamespacedDimensionStringPassesThroughUnchanged(@TempDir Path tempDir) throws IOException {
69+
assertEquals("minecraft:overworld", load(tempDir, "minecraft:overworld"));
70+
}
71+
72+
@Test
73+
void loadSignEntriesBacksUpTheOriginalFileBeforeReturning(@TempDir Path tempDir) throws IOException {
74+
var path = tempDir.resolve("signs.json").toString();
75+
var entry = new SignEntryV2(
76+
new SignEntryKey(0, 64, 0, "overworld"),
77+
"player-1",
78+
new SignLinesParseResultV2(MarkerTypeV2.POI, "Town Hall", "Town Hall"),
79+
new SignLinesParseResultV2(null, "", ""));
80+
var content = GSON.toJson(new SignEntryV2[]{entry});
81+
Files.writeString(Path.of(path), content, StandardCharsets.UTF_8);
82+
83+
Version1SignEntryLoader.loadSignEntries(path, content, POI_GROUP, GSON);
84+
85+
var backup = Path.of(path + ".v1.bak");
86+
assertTrue(Files.exists(backup), "the original V1 file should be backed up");
87+
assertEquals(content, Files.readString(backup));
88+
}
89+
}

0 commit comments

Comments
 (0)