Address review on the test framework

- The registry is a Suite, created around the environment factory it runs its
  tests with, so the plugin's env.lua reads as: build environments, hand the
  test files a suite of them. The suite returned there becomes `...` in each
  test file, which is now said where it happens.
- pass and fail are the primitives every other check goes through. eq and
  deep_eq assert rather than compare, failing with both values in the detail.
- Stubs are extended through extend_requires, extend_script, extend_game and
  extend_defines, a recursive merge which raises when a value already exists,
  rather than by mutating the tables. The strict labels carry the factorio
  class names.
- The stubs record registered metatables and can save and load the script
  data the way factorio does: functions are refused and only registered
  metatables survive. env.save_load() uses it, and a new on_load test shows
  role methods and held roles surviving the round trip, which only passes
  because the module registers its metatable.
- The names helper moved out of the shared framework, it is role specific.
- Tests are named after the function or method they cover, on both sides:
  the lua tests read as "role:assign applies locally and is sent", and the
  javascript files wrap their tests in the class they exercise with subtests
  per method. module.test.js is control.test.js, after the file it covers.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

test/lua/framework.lua

@@ -1,15 +1,30 @@
 --[[-- Test framework for plugin module tests
-Test files declare named tests with `Test.test(name, function(env) ... end)`.
-The runner creates a fresh environment for every test, so tests are
-independent and can not leak state into each other.
+A suite is a collection of named tests. Test files declare them with
+`Suite.test(name, function(env) ... end)`, naming each test after the function
+or method it covers. Every test function receives a fresh environment from the
+factory the suite was created with, so tests are independent and can not leak
+state into each other.
 ]]
 
 local Framework = {}
 
---- Create a test registry, one is made per test file
-function Framework.new()
-    --- @class Test
-    local Test = {
+--- Turn a value into a string for failure messages
+local function repr(value, depth)
+    if type(value) ~= "table" then return tostring(value) end
+    if depth > 3 then return "{...}" end
+
+    local parts = {}
+    for key, entry in pairs(value) do
+        parts[#parts + 1] = tostring(key) .. " = " .. repr(entry, depth + 1)
+    end
+    return "{ " .. table.concat(parts, ", ") .. " }"
+end
+
+--- Create a suite of tests, one is made per test file
+--- @param make_env fun(): table Creates the environment given to each test
+function Framework.new_suite(make_env)
+    --- @class Suite
+    local Suite = {
         tests = {},   -- { name, fn } in declaration order
         results = {}, -- { name, ok, detail? } accumulated across tests
     }
@@ -19,37 +34,61 @@ function Framework.new()
     --- Declare a named test, its function receives a fresh environment
     --- @param name string
     --- @param fn fun(env: table)
-    function Test.test(name, fn)
-        Test.tests[#Test.tests + 1] = { name = name, fn = fn }
+    function Suite.test(name, fn)
+        Suite.tests[#Suite.tests + 1] = { name = name, fn = fn }
     end
 
-    --- Record one check within the current test
-    --- @param ok any Truthy when the check passed
+    --- Record a passing check within the current test
+    --- @param name string
+    function Suite.pass(name)
+        Suite.results[#Suite.results + 1] = {
+            name = current_test and (current_test .. ": " .. name) or name,
+            ok = true,
+        }
+    end
+
+    --- Record a failing check within the current test
     --- @param name string
     --- @param detail string?
-    function Test.check(ok, name, detail)
-        Test.results[#Test.results + 1] = {
+    function Suite.fail(name, detail)
+        Suite.results[#Suite.results + 1] = {
             name = current_test and (current_test .. ": " .. name) or name,
-            ok = not not ok,
+            ok = false,
             detail = detail,
         }
     end
 
-    --- Shallow array equality
-    function Test.eq(a, b)
-        if type(a) ~= "table" or type(b) ~= "table" then return a == b end
-        if #a ~= #b then return false end
-        for i = 1, #a do
-            if a[i] ~= b[i] then return false end
+    --- Check a condition
+    --- @param ok any Truthy when the check passed
+    --- @param name string
+    --- @param detail string?
+    function Suite.check(ok, name, detail)
+        if ok then
+            Suite.pass(name)
+        else
+            Suite.fail(name, detail)
         end
-        return true
+    end
+
+    --- Check two values or arrays are shallowly equal
+    function Suite.eq(a, b, name)
+        local equal
+        if type(a) ~= "table" or type(b) ~= "table" then
+            equal = a == b
+        else
+            equal = #a == #b
+            for i = 1, #a do
+                if a[i] ~= b[i] then equal = false break end
+            end
+        end
+        Suite.check(equal, name, ("%s ~= %s"):format(repr(a, 1), repr(b, 1)))
     end
 
     --- Recursive table equality, keys checked from both sides
-    function Test.deep_eq(a, b)
+    local function deep_equal(a, b)
         if type(a) ~= "table" or type(b) ~= "table" then return a == b end
         for key, value in pairs(a) do
-            if not Test.deep_eq(value, b[key]) then return false end
+            if not deep_equal(value, b[key]) then return false end
         end
         for key in pairs(b) do
             if a[key] == nil then return false end
@@ -57,38 +96,31 @@ function Framework.new()
         return true
     end
 
-    --- Names of an array of roles
-    function Test.names(roles)
-        local rtn = {}
-        for index, role in ipairs(roles) do
-            rtn[index] = role.name
-        end
-        return rtn
+    --- Check two values are equal, comparing tables recursively
+    function Suite.deep_eq(a, b, name)
+        Suite.check(deep_equal(a, b), name, ("%s ~= %s"):format(repr(a, 1), repr(b, 1)))
     end
 
     --- Sorted copy of an array of strings
-    function Test.sorted(list)
+    function Suite.sorted(list)
         local rtn = {}
         for index, value in ipairs(list) do rtn[index] = value end
         table.sort(rtn)
         return rtn
     end
 
-    --- Run every declared test against a fresh environment
-    --- @param make_env fun(): table
-    function Test.run(make_env)
-        for _, test in ipairs(Test.tests) do
+    --- Run every declared test, each against a fresh environment, and return
+    --- the results as JSON for the javascript side
+    function Suite.run()
+        for _, test in ipairs(Suite.tests) do
             current_test = test.name
             local ok, err = pcall(test.fn, make_env())
             if not ok then
-                Test.check(false, "did not error", tostring(err))
+                Suite.fail("did not error", tostring(err))
             end
         end
         current_test = nil
-    end
 
-    --- Encode the results as JSON for the javascript side
-    function Test.results_json()
         local function escape(value)
             return (value:gsub('[%c"\\]', function(c)
                 return string.format("\\u%04x", c:byte())
@@ -96,14 +128,14 @@ function Framework.new()
         end
 
         local parts = {}
-        for index, result in ipairs(Test.results) do
+        for index, result in ipairs(Suite.results) do
             local detail = result.detail and string.format(',"detail":"%s"', escape(result.detail)) or ""
             parts[index] = string.format('{"name":"%s","ok":%s%s}', escape(result.name), tostring(result.ok), detail)
         end
         return "[" .. table.concat(parts, ",") .. "]"
     end
 
-    return Test
+    return Suite
 end
 
 return Framework

test/lua/stubs.lua

@@ -3,8 +3,8 @@ Provides the surface a clusterio module touches, recording what the module does
 to it. Every stub raises an error when an unimplemented property is read, which
 mirrors the game api and catches mistakes in tests early.
 
-Plugins compose these from their own test/lua/env.lua, adding stubs of their
-own through `stubs.requires` and `Stubs.strict`.
+Plugins compose these from their own test/lua/env.lua, adding their own stubs
+with the extend methods rather than by mutating the tables directly.
 ]]
 
 local Stubs = {}
@@ -29,6 +29,25 @@ function Stubs.strict(name, tbl, allowed_nil)
     })
 end
 
+--- Merge extra into target recursively, raising when a value already exists
+--- @generic T : table
+--- @param target T
+--- @param extra table
+--- @return T
+function Stubs.extend(target, extra)
+    for key, value in pairs(extra) do
+        local existing = rawget(target, key)
+        if existing == nil then
+            rawset(target, key, value)
+        elseif type(existing) == "table" and type(value) == "table" then
+            Stubs.extend(existing, value)
+        else
+            error("Stub already implements: " .. tostring(key), 2)
+        end
+    end
+    return target
+end
+
 --- Create an independent set of stubs, installed as the lua globals
 function Stubs.new()
     local stubs = {
@@ -39,7 +58,8 @@ function Stubs.new()
     }
 
     local next_event_id = 100
-    script = Stubs.strict("script", {
+    local registered_metatables = {} --- @type table<table, true>
+    script = Stubs.strict("LuaBootstrap", {
         generate_event_name = function()
             next_event_id = next_event_id + 1
             return next_event_id
@@ -47,7 +67,9 @@ function Stubs.new()
         raise_event = function(id, data)
             stubs.events[#stubs.events + 1] = { id = id, data = data }
         end,
-        register_metatable = function() end,
+        register_metatable = function(_, metatable)
+            registered_metatables[metatable] = true
+        end,
     })
 
     defines = Stubs.strict("defines", {
@@ -58,7 +80,7 @@ function Stubs.new()
     })
 
     local players_by_name, players_by_index, connected = {}, {}, {}
-    game = Stubs.strict("game", {
+    game = Stubs.strict("LuaGameScript", {
         tick = 1,
         players = players_by_name,
         connected_players = connected,
@@ -87,10 +109,10 @@ function Stubs.new()
     end
 
     --- A player object which represents the server
-    stubs.server = Stubs.strict("server player", { index = 0, name = "<server>" })
+    stubs.server = Stubs.strict("LuaPlayer <server>", { index = 0, name = "<server>" })
 
-    --- Modules resolved by the stubbed require, extend before loading the module
-    stubs.requires = {
+    --- Modules resolved by the stubbed require, add to them with extend_requires
+    local requires = {
         ["modules/clusterio/api"] = Stubs.strict("clusterio api", {
             send_json = function(channel, data)
                 stubs.sent[#stubs.sent + 1] = { channel = channel, data = data }
@@ -104,7 +126,44 @@ function Stubs.new()
         }),
     }
     require = function(name)
-        return assert(stubs.requires[name], "Unexpected require: " .. name)
+        return assert(rawget(requires, name), "Unexpected require: " .. name)
+    end
+
+    --- Add modules or properties to the stubbed require
+    function stubs.extend_requires(extra) return Stubs.extend(requires, extra) end
+
+    --- Add properties to the script, game, and defines globals
+    function stubs.extend_script(extra) return Stubs.extend(script, extra) end
+    function stubs.extend_game(extra) return Stubs.extend(game, extra) end
+    function stubs.extend_defines(extra) return Stubs.extend(defines, extra) end
+
+    --- Copy a value the way factorio saves script data: functions are refused
+    --- and only metatables registered with script.register_metatable survive
+    local function save_load_copy(value, copies)
+        if type(value) == "function" then
+            error("Functions can not be stored in script data", 0)
+        end
+        if type(value) ~= "table" then return value end
+        if copies[value] then return copies[value] end
+
+        local copy = {}
+        copies[value] = copy
+        for key, entry in pairs(value) do
+            copy[save_load_copy(key, copies)] = save_load_copy(entry, copies)
+        end
+
+        local metatable = getmetatable(value)
+        if metatable ~= nil and registered_metatables[metatable] then
+            setmetatable(copy, metatable)
+        end
+        return copy
+    end
+
+    --- Replace the script data with a copy of itself as if the map was saved
+    --- and loaded, the module's on_load handler should be called afterwards
+    function stubs.save_load()
+        local compat = requires["modules/clusterio/compat"]
+        compat.script_data = save_load_copy(compat.script_data, {})
     end
 
     --- Forget everything recorded so far