Make the determinism failure say what differed

.github/workflows/build.yml

@@ -127,6 +127,21 @@ jobs:
           check_name: Test Results (${{ matrix.os }})
           fail_on_failure: false
 
+      # luaOutputIsDeterministicForGenericOverrideSlots compiles one program twice and keeps both
+      # scripts when they differ. They are written to the runner's filesystem, so without this the
+      # evidence goes with the runner and the failure is as unactionable as it was before it kept
+      # anything. Most failures are not this test, hence if-no-files-found: ignore.
+      - name: Upload determinism scripts (on failure)
+        if: failure()
+        uses: actions/upload-artifact@v4
+        with:
+          name: determinism-scripts-${{ matrix.os }}
+          path: |
+            de.peeeq.wurstscript/test-output/determinism-first.lua
+            de.peeeq.wurstscript/test-output/determinism-second.lua
+          if-no-files-found: ignore
+          retention-days: 14
+
       - name: Upload packaged artifact (per-OS)
         uses: actions/upload-artifact@v4
         with:

BACKLOG.md

@@ -125,22 +125,27 @@ itself, and one gap in what the suite can see.
     this stops being dead weight, because the cost of getting it wrong is a real mis-binding while
     the cost of leaving it is one unused table key per specialised class.
 
-24. **`luaOutputIsDeterministicForGenericOverrideSlots` fails intermittently.** It failed once on
-    Windows CI and passed on a re-run of the same commit, having blocked an unrelated pull request
+24. **`luaOutputIsDeterministicForGenericOverrideSlots` failed once and has not since.** It failed
+    on Windows CI and passed on a re-run of the same commit, having blocked an unrelated pull request
     in between.
 
     Do not weaken the assertion. It compiles one repro twice and compares the output byte for byte,
-    so an intermittent mismatch is evidence of intermittent nondeterminism in Lua emission, which is
-    exactly what it exists to catch — a re-run passing says the nondeterminism is intermittent, not
-    that the test is at fault. Calling it flaky was too quick.
-
-    Diagnose it instead: capture both outputs on a failing run and diff them, and rule out harness
-    interference rather than assuming it. The two compiles do start from the same cache state, but
-    that comes from two separate resets: `WurstScriptTest`'s `@BeforeMethod` clears before the
-    first, and the explicit `GlobalCaches.clearAll()` between the compilations inside the test
-    clears before the second. Both are load bearing — remove either and the comparison stops being
-    between equal starting states, which would invalidate the conclusion rather than explain the
-    failure.
+    so a mismatch is evidence of nondeterminism in Lua emission, which is what it exists to catch.
+
+    **Not reproduced.** 250 compiles of that repro in one JVM, caches cleared between each, came out
+    byte-identical. Sources of hash-ordered iteration in the emission path were read rather than
+    guessed at: `TypeId.calculate` sorts by name and package, `createMethods` groups through a
+    `TreeMap`, `assignDispatchAliases` collects into a `TreeSet` and iterates a list, and
+    `collectSuperClasses` uses its set only to mark what it has seen. None of those can vary.
+
+    So whatever differs is either rarer than one in 250, or comes from something the local run does
+    not vary — a different core count changing the fork layout, memory pressure, or the interpreter
+    build on that runner.
+
+    What changed meanwhile is that the failure now carries evidence: both scripts are written beside
+    the test output and the first differing lines are named with their numbers. The one occurrence so
+    far produced nothing to work from, which is why it cost a re-run and no diagnosis. The next one
+    will say what differed.
 
 12. **Standing item, never finished.** When nothing above is left, find the next thing worth
     doing and add it here rather than stopping. Good sources, in order: a test that would have

de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/DeterminismDiffReportTest.java

@@ -0,0 +1,88 @@
+package tests.wurstscript.tests;
+
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertNotEquals;
+import static org.testng.Assert.assertTrue;
+
+/**
+ * Covers the summary the determinism test prints when it fails.
+ * <p>
+ * The failure it describes has happened once and is not reproducible on demand, so the summary is
+ * the only thing that will be read when it happens again. That makes it worth testing on its own:
+ * a description which misreports what changed is worse than none, because it sends the next person
+ * looking in the wrong place.
+ */
+public class DeterminismDiffReportTest {
+
+    /**
+     * A block appearing in one script and not the other reads as one addition. Comparing equal
+     * indexes instead would report every line after the insertion as differing, which is the case
+     * emission-order nondeterminism actually produces.
+     */
+    @Test
+    public void anInsertedBlockIsOneAdditionRatherThanEverythingAfterIt() {
+        String first = "a\nb\nc\nd\ne\nf\ng\nh\n";
+        String second = "a\nb\nINSERTED\nc\nd\ne\nf\ng\nh\n";
+
+        String report = LuaTranslationTests.describeFirstDifferences(first, second);
+
+        assertTrue(report.contains("0 line(s) only in the first, 1 only in the second"),
+            "one inserted line should read as one addition:\n" + report);
+        assertTrue(report.contains("INSERTED"), "the added line should be named:\n" + report);
+        assertTrue(report.contains("line 3"), "the added line's number should be given:\n" + report);
+    }
+
+    /** A block moved rather than inserted reads as one removal and one addition, not a cascade. */
+    @Test
+    public void aMovedLineIsOneRemovalAndOneAddition() {
+        String first = "one\nmoved\ntwo\nthree\nfour\n";
+        String second = "one\ntwo\nthree\nmoved\nfour\n";
+
+        String report = LuaTranslationTests.describeFirstDifferences(first, second);
+
+        assertTrue(report.contains("1 line(s) only in the first, 1 only in the second"),
+            "a moved line should read as one removal and one addition:\n" + report);
+    }
+
+    /** Replacing a line in place is a removal and an addition at the same position. */
+    @Test
+    public void aReplacedLineNamesBothVersions() {
+        String first = "x\nbefore\nz\n";
+        String second = "x\nafter\nz\n";
+
+        String report = LuaTranslationTests.describeFirstDifferences(first, second);
+
+        assertTrue(report.contains("before"), "the first version should be named:\n" + report);
+        assertTrue(report.contains("after"), "the second version should be named:\n" + report);
+    }
+
+    /**
+     * Two scripts whose lines all match but which are not equal differ in how the lines end. The
+     * caller only asks after finding them unequal, so passing identical strings would test a state
+     * production never reaches — this passes CRLF against LF, which it can.
+     */
+    @Test
+    public void differingOnlyInLineEndingsSaysSoRatherThanListingEveryLine() {
+        String crlf = "alpha\r\nbeta\r\ngamma\r\n";
+        String lf = "alpha\nbeta\ngamma\n";
+        assertNotEquals(crlf, lf, "the two inputs must be unequal for this to mean anything");
+
+        String report = LuaTranslationTests.describeFirstDifferences(crlf, lf);
+
+        assertTrue(report.contains("line terminators"),
+            "line endings should be named rather than every line reported as changed:\n" + report);
+        assertFalse(report.contains("only in the first"),
+            "no line should be reported as removed:\n" + report);
+    }
+
+    /** Bytes after the last line reach the same branch: every line matches, the scripts do not. */
+    @Test
+    public void trailingBytesReachTheSameCase() {
+        String report = LuaTranslationTests.describeFirstDifferences("a\nb\n", "a\nb");
+
+        assertTrue(report.contains("line terminators") || report.contains("only in the"),
+            "a trailing difference should be described one way or the other:\n" + report);
+    }
+}

de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/LuaTranslationTests.java

@@ -1418,7 +1418,102 @@ public void luaOutputIsDeterministicForGenericOverrideSlots() throws IOException
         test().testLua(true).compilationUnits(genericOverrideReproUnits());
         String second = Files.toString(new File("test-output/lua/LuaTranslationTests_luaOutputIsDeterministicForGenericOverrideSlots.lua"), Charsets.UTF_8);
 
-        assertEquals(first, second);
+        if (!first.equals(second)) {
+            // This has failed once on CI and not since, and 250 compiles in one JVM did not
+            // reproduce it. A bare "expected X but got Y" over two whole scripts is unreadable and
+            // the run's output is gone by the time anyone looks, so the failure carries what it
+            // takes to act on: both scripts kept beside the test output, and the differing lines
+            // named. Without this the next occurrence is as unactionable as the first.
+            File firstFile = new File(TEST_OUTPUT_PATH, "determinism-first.lua");
+            File secondFile = new File(TEST_OUTPUT_PATH, "determinism-second.lua");
+            Files.write(first.getBytes(Charsets.UTF_8), firstFile);
+            Files.write(second.getBytes(Charsets.UTF_8), secondFile);
+            fail("the same program compiled to different Lua twice in one run."
+                + "\n" + describeFirstDifferences(first, second)
+                + "\nboth kept at " + firstFile.getPath() + " and " + secondFile.getPath());
+        }
+    }
+
+    /**
+     * What changed between the two scripts, aligned rather than compared line by line.
+     * <p>
+     * Comparing equal indexes is not a diff: emission order moving a block shifts every line after
+     * it, so the count becomes "everything from here down" and the listed pairs are unrelated. That
+     * is the shape this diagnostic exists to investigate, so it is the shape it has to describe.
+     * Aligned on the longest common subsequence, a moved block reads as one removal and one addition.
+     * <p>
+     * The scripts are also uploaded as an artifact on a failing CI run, but the message has to stand
+     * on its own: an artifact needs fetching, and the check is what gets read first. Bounded so a
+     * wholesale difference does not bury the report, with the totals stated either way.
+     */
+    /**
+     * Splits on either terminator, so a CRLF script aligns against an LF one line for line. Splitting
+     * on "\n" alone leaves the carriage return in the line text, which makes every line of a CRLF
+     * script differ from its LF counterpart and buries the actual difference.
+     */
+    private static final String NEWLINE_RE = "\r?\n";
+    private static final char NEWLINE = '\n';
+
+    static String describeFirstDifferences(String first, String second) {
+        final int reportLimit = 40;
+        String[] a = first.split(NEWLINE_RE, -1);
+        String[] b = second.split(NEWLINE_RE, -1);
+
+        // O(n*m) in memory, so a pathological pair falls back to reporting the sizes rather than
+        // exhausting the worker. The scripts this compares are a few hundred lines.
+        if ((long) a.length * b.length > 4_000_000L) {
+            return "  too large to align: " + a.length + " lines against " + b.length;
+        }
+
+        int[][] common = new int[a.length + 1][b.length + 1];
+        for (int i = a.length - 1; i >= 0; i--) {
+            for (int j = b.length - 1; j >= 0; j--) {
+                common[i][j] = a[i].equals(b[j])
+                    ? common[i + 1][j + 1] + 1
+                    : Math.max(common[i + 1][j], common[i][j + 1]);
+            }
+        }
+
+        List<String> entries = new ArrayList<>();
+        int removed = 0;
+        int added = 0;
+        int i = 0;
+        int j = 0;
+        while (i < a.length || j < b.length) {
+            if (i < a.length && j < b.length && a[i].equals(b[j])) {
+                i++;
+                j++;
+            } else if (j >= b.length || (i < a.length && common[i + 1][j] >= common[i][j + 1])) {
+                removed++;
+                if (entries.size() < reportLimit) {
+                    entries.add("    only in first, line " + (i + 1) + ":  " + a[i]);
+                }
+                i++;
+            } else {
+                added++;
+                if (entries.size() < reportLimit) {
+                    entries.add("    only in second, line " + (j + 1) + ": " + b[j]);
+                }
+                j++;
+            }
+        }
+
+        if (removed == 0 && added == 0) {
+            // Reached because the split accepts either terminator: two scripts whose lines all match
+            // and which are still unequal differ in how those lines end, or in bytes after the last
+            // one. Worth saying plainly rather than reporting every line as changed, which is what
+            // splitting on "\n" alone did - it left the carriage returns in the line text.
+            return "  every line matches, so the two differ only in line terminators or trailing"
+                + " bytes: " + first.length() + " characters against " + second.length();
+        }
+        StringBuilder sb = new StringBuilder();
+        sb.append("  ").append(removed).append(" line(s) only in the first, ")
+            .append(added).append(" only in the second");
+        sb.append(removed + added > entries.size() ? ", first " + entries.size() + ":" + NEWLINE : ":" + NEWLINE);
+        for (String entry : entries) {
+            sb.append(entry).append(NEWLINE);
+        }
+        return sb.toString();
     }
 
     @Test