From 3cb25b3525cdad7486ce1e0222ceb7365f68c9d6 Mon Sep 17 00:00:00 2001 From: Jonathan Coates Date: Tue, 27 Apr 2021 22:24:28 +0100 Subject: [PATCH] Various VM tests These are largely copied across from Cobalt's test suite, with some minor tweaks. It actually exposed one bug in Cobalt, which is pretty nice. One interesting thing from the coroutine tests, is that Lua 5.4 (and one assumes 5.2/5.3) doesn't allow yielding from within the error handler of xpcall - I rather thought it might. This doesn't add any of the PUC Lua tests yet - I got a little distracted. Also: - Allow skipping "keyword" tests, in the style of busted. This is implemented on the Java side for now. - Fix a bug with os.date("%I", _) not being 2 characters wide. --- build.gradle | 1 + .../computercraft/core/apis/LuaDateTime.java | 2 +- .../core/ComputerTestDelegate.java | 33 +- src/test/resources/test-rom/mcfly.lua | 3 + .../resources/test-rom/spec/apis/os_spec.lua | 5 + .../resources/test-rom/spec/lua/README.md | 19 + .../test-rom/spec/lua/coroutine_spec.lua | 330 ++++++++++++++++++ .../test-rom/spec/lua/timeout_spec.lua | 17 + .../resources/test-rom/spec/lua/vm_spec.lua | 9 + 9 files changed, 409 insertions(+), 10 deletions(-) create mode 100644 src/test/resources/test-rom/spec/lua/README.md create mode 100644 src/test/resources/test-rom/spec/lua/coroutine_spec.lua create mode 100644 src/test/resources/test-rom/spec/lua/timeout_spec.lua create mode 100644 src/test/resources/test-rom/spec/lua/vm_spec.lua diff --git a/build.gradle b/build.gradle index 2ede63c40..6611a8eb7 100644 --- a/build.gradle +++ b/build.gradle @@ -104,6 +104,7 @@ accessTransformer file('src/main/resources/META-INF/accesstransformer.cfg') } repositories { + mavenLocal() mavenCentral() maven { name "SquidDev" diff --git a/src/main/java/dan200/computercraft/core/apis/LuaDateTime.java b/src/main/java/dan200/computercraft/core/apis/LuaDateTime.java index 9ad29ad4c..3293d2b7c 100644 --- a/src/main/java/dan200/computercraft/core/apis/LuaDateTime.java +++ b/src/main/java/dan200/computercraft/core/apis/LuaDateTime.java @@ -89,7 +89,7 @@ static void format( DateTimeFormatterBuilder formatter, String format, ZoneOffse formatter.appendValue( ChronoField.HOUR_OF_DAY, 2 ); break; case 'I': - formatter.appendValue( ChronoField.HOUR_OF_AMPM ); + formatter.appendValue( ChronoField.HOUR_OF_AMPM, 2 ); break; case 'j': formatter.appendValue( ChronoField.DAY_OF_YEAR, 3 ); diff --git a/src/test/java/dan200/computercraft/core/ComputerTestDelegate.java b/src/test/java/dan200/computercraft/core/ComputerTestDelegate.java index 16eab74da..de5b621bf 100644 --- a/src/test/java/dan200/computercraft/core/ComputerTestDelegate.java +++ b/src/test/java/dan200/computercraft/core/ComputerTestDelegate.java @@ -42,6 +42,8 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.Condition; import java.util.concurrent.locks.ReentrantLock; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Stream; import static dan200.computercraft.api.lua.LuaValues.getType; @@ -67,6 +69,12 @@ public class ComputerTestDelegate private static final long TIMEOUT = TimeUnit.SECONDS.toNanos( 10 ); + private static final Set SKIP_KEYWORDS = new HashSet<>( + Arrays.asList( System.getProperty( "cc.skip_keywords", "" ).split( "," ) ) + ); + + private static final Pattern KEYWORD = Pattern.compile( ":([a-z_]+)" ); + private final ReentrantLock lock = new ReentrantLock(); private Computer computer; @@ -212,18 +220,23 @@ DynamicNodeBuilder get( String name ) void runs( String name, Executable executor ) { - DynamicNodeBuilder child = children.get( name ); - int id = 0; - while( child != null ) - { - id++; - String subName = name + "_" + id; - child = children.get( subName ); - } + if( this.executor != null ) throw new IllegalStateException( name + " is leaf node" ); + if( children.containsKey( name ) ) throw new IllegalStateException( "Duplicate key for " + name ); children.put( name, new DynamicNodeBuilder( name, executor ) ); } + boolean isActive() + { + Matcher matcher = KEYWORD.matcher( name ); + while( matcher.find() ) + { + if( SKIP_KEYWORDS.contains( matcher.group( 1 ) ) ) return false; + } + + return true; + } + DynamicNode build() { return executor == null @@ -233,7 +246,9 @@ DynamicNode build() Stream buildChildren() { - return children.values().stream().map( DynamicNodeBuilder::build ); + return children.values().stream() + .filter( DynamicNodeBuilder::isActive ) + .map( DynamicNodeBuilder::build ); } } diff --git a/src/test/resources/test-rom/mcfly.lua b/src/test/resources/test-rom/mcfly.lua index ddd4f46b3..6c3bbc1df 100644 --- a/src/test/resources/test-rom/mcfly.lua +++ b/src/test/resources/test-rom/mcfly.lua @@ -612,6 +612,9 @@ local function do_run(test) elseif test.action then local state = push_state() + -- Flush the event queue and ensure we're running with 0 timeout. + os.queueEvent("start_test") os.pullEvent("start_test") + local ok ok, err = try(test.action) status = ok and "pass" or (err.fail and "fail" or "error") diff --git a/src/test/resources/test-rom/spec/apis/os_spec.lua b/src/test/resources/test-rom/spec/apis/os_spec.lua index 7fc1add53..c04c22a48 100644 --- a/src/test/resources/test-rom/spec/apis/os_spec.lua +++ b/src/test/resources/test-rom/spec/apis/os_spec.lua @@ -108,6 +108,11 @@ describe("The os library", function() error("Non letter character in zone: " .. zone) end end) + + local t2 = os.time { year = 2000, month = 10, day = 1, hour = 3, min = 12, sec = 17 } + it("for code '%I' #2", function() + expect(os.date("%I", t2)):eq("03") + end) end) end) diff --git a/src/test/resources/test-rom/spec/lua/README.md b/src/test/resources/test-rom/spec/lua/README.md new file mode 100644 index 000000000..9bd6c61a0 --- /dev/null +++ b/src/test/resources/test-rom/spec/lua/README.md @@ -0,0 +1,19 @@ +# Lua VM tests + +Unlike the rest of the test suites, the code in this folder doesn't test any +(well, much) CC code. Instead, it ensures that the Lua VM behaves in a way that +we expect. + +The VM that CC uses (LuaJ and later Cobalt) does not really conform to any one +version of Lua, instead supporting a myriad of features from Lua 5.1 to 5.3 (and +not always accurately). These tests attempt to pin down what behaviour is +required for a well behaved emulator. + + +These tests are split into several folders: + - `/` Tests for CC specific functionality, based on Cobalt/Lua quirks or needs + of CC. + - `puc`: Tests derived from the [PUC Lua test suite][puc-tests]. + + +[puc-tests]: https://www.lua.org/tests/ "Lua: test suites" diff --git a/src/test/resources/test-rom/spec/lua/coroutine_spec.lua b/src/test/resources/test-rom/spec/lua/coroutine_spec.lua new file mode 100644 index 000000000..6be0826c8 --- /dev/null +++ b/src/test/resources/test-rom/spec/lua/coroutine_spec.lua @@ -0,0 +1,330 @@ +describe("Coroutines", function() + local function assert_resume(ok, ...) + if ok then return table.pack(...) end + error(..., 0) + end + + --- Run a function in a coroutine, "echoing" the yielded value back as the resumption value. + local function coroutine_echo(f) + local co = coroutine.create(f) + local result = {n = 0} + while coroutine.status(co) ~= "dead" do + result = assert_resume(coroutine.resume(co, table.unpack(result, 1, result.n))) + end + + return table.unpack(result, 1, result.n) + end + + describe("allow yielding", function() + --[[ + Tests for some non-standard yield locations. I'm not saying that users + /should/ use this, but it's useful for us to allow it in order to suspend the + VM in arbitrary locations. + + Cobalt does support this within load too, but that's unlikely to be supported + in the future. + + These tests were split over about 7 files in Cobalt and are in one massive one + in this test suite. Sorry. + ]] + + it("within debug hooks", function() + coroutine_echo(function() + local counts = { call = 0, ['return'] = 0, count = 0, line = 0 } + + debug.sethook(function(kind) + counts[kind] = (counts[kind] or 0) + 1 + expect(coroutine.yield(kind)):eq(kind) + end, "crl", 1) + + expect(string.gsub("xyz", "x", "z")):eq("zyz") + expect(pcall(function() + local x = 0 + for i = 1, 5 do x = x + i end + end)):eq(true) + + debug.sethook(nil) + + -- These numbers are going to vary beyond the different VMs a + -- little. As long as they're non-0, it's all fine. + expect(counts.call):ne(0) + expect(counts['return']):ne(0) + expect(counts.count):ne(0) + expect(counts.line):ne(0) + end) + end) + + it("within string.gsub", function() + local result, count = coroutine_echo(function() + return ("hello world"):gsub("%w", function(entry) + local x = coroutine.yield(entry) + return x:upper() + end) + end) + + expect(result):eq("HELLO WORLD") + expect(count):eq(10) + end) + + describe("within pcall", function() + it("with no error", function() + local ok, a, b, c = coroutine_echo(function() + return pcall(function() + local a, b, c = coroutine.yield(1, 2, 3) + return a, b, c + end) + end) + + expect(ok):eq(true) + expect({ a, b, c }):same { 1, 2, 3 } + end) + + it("with an error", function() + local ok, msg = coroutine_echo(function() + return pcall(function() + local a, b, c = coroutine.yield(1, 2, 3) + expect({ a, b, c }):same { 1, 2, 3 } + error("Error message", 0) + end) + end) + + expect(ok):eq(false) + expect(msg):eq("Error message") + end) + end) + + it("within table.foreach", function() + coroutine_echo(function() + local x = { 3, "foo", 4, 1 } + local idx = 1 + table.foreach(x, function(key, val) + expect(key):eq(idx) + expect(val):eq(x[idx]) + expect(coroutine.yield(val)):eq(val) + + idx = idx + 1 + end) + end) + end) + + it("within table.foreachi", function() + coroutine_echo(function() + local x = { 3, "foo", 4, 1 } + local idx = 1 + table.foreachi(x, function(key, val) + expect(key):eq(idx) + expect(val):eq(x[idx]) + expect(coroutine.yield(val)):eq(val) + + idx = idx + 1 + end) + end) + end) + + describe("within table.sort", function() + it("with a yielding comparator", function() + coroutine_echo(function() + local x = { 32, 2, 4, 13 } + table.sort(x, function(a, b) + local x, y = coroutine.yield(a, b) + expect(x):eq(a) + expect(y):eq(b) + + return a < b + end) + + expect(x[1]):eq(2) + expect(x[2]):eq(4) + expect(x[3]):eq(13) + expect(x[4]):eq(32) + end) + end) + + it("within a yielding metatable comparator", function() + local meta = { + __lt = function(a, b) + local x, y = coroutine.yield(a, b) + expect(x):eq(a) + expect(y):eq(b) + + return a.x < b.x + end + } + + local function create(val) return setmetatable({ x = val }, meta) end + + coroutine_echo(function() + local x = { create(32), create(2), create(4), create(13) } + table.sort(x) + + expect(x[1].x):eq(2) + expect(x[2].x):eq(4) + expect(x[3].x):eq(13) + expect(x[4].x):eq(32) + end) + end) + end) + + describe("within xpcall", function() + it("within the main function", function() + -- Ensure that yielding within a xpcall works as expected + coroutine_echo(function() + local ok, a, b, c = xpcall(function() + return coroutine.yield(1, 2, 3) + end, function(msg) return msg .. "!" end) + + expect(true):eq(ok) + expect(1):eq(a) + expect(2):eq(b) + expect(3):eq(c) + end) + end) + + it("within the main function (with an error)", function() + coroutine_echo(function() + local ok, msg = xpcall(function() + local a, b, c = coroutine.yield(1, 2, 3) + expect(1):eq(a) + expect(2):eq(b) + expect(3):eq(c) + + error("Error message", 0) + end, function(msg) return msg .. "!" end) + + expect(false):eq(ok) + expect("Error message!"):eq(msg) + end) + end) + + it("with an error in the error handler", function() + coroutine_echo(function() + local ok, msg = xpcall(function() + local a, b, c = coroutine.yield(1, 2, 3) + expect(1):eq(a) + expect(2):eq(b) + expect(3):eq(c) + + error("Error message") + end, function(msg) error(msg) end) + + expect(false):eq(ok) + expect("error in error handling"):eq(msg) + end) + end) + + it("within the error handler", function() + coroutine_echo(function() + local ok, msg = xpcall(function() + local a, b, c = coroutine.yield(1, 2, 3) + expect(1):eq(a) + expect(2):eq(b) + expect(3):eq(c) + + error("Error message", 0) + end, function(msg) + return coroutine.yield(msg) .. "!" + end) + + expect(false):eq(ok) + expect("Error message!"):eq(msg) + end) + end) + + it("within the error handler with an error", function() + coroutine_echo(function() + local ok, msg = xpcall(function() + local a, b, c = coroutine.yield(1, 2, 3) + expect(1):eq(a) + expect(2):eq(b) + expect(3):eq(c) + + error("Error message", 0) + end, function(msg) + coroutine.yield(msg) + error("nope") + end) + + expect(false):eq(ok) + expect("error in error handling"):eq(msg) + end) + end) + end) + + it("within metamethods", function() + local create, ops + create = function(val) return setmetatable({ x = val }, ops) end + ops = { + __add = function(x, y) + local a, b = coroutine.yield(x, y) + return create(a.x + b.x) + end, + __div = function(x, y) + local a, b = coroutine.yield(x, y) + return create(a.x / b.x) + end, + __concat = function(x, y) + local a, b = coroutine.yield(x, y) + return create(a.x .. b.x) + end, + __eq = function(x, y) + local a, b = coroutine.yield(x, y) + return a.x == b.x + end, + __lt = function(x, y) + local a, b = coroutine.yield(x, y) + return a.x < b.x + end, + __index = function(tbl, key) + local res = coroutine.yield(key) + return res:upper() + end, + __newindex = function(tbl, key, val) + local rKey, rVal = coroutine.yield(key, val) + rawset(tbl, rKey, rVal .. "!") + end, + } + + local varA = create(2) + local varB = create(3) + + coroutine_echo(function() + expect(5):eq((varA + varB).x) + expect(5):eq((varB + varA).x) + expect(4):eq((varA + varA).x) + expect(6):eq((varB + varB).x) + + expect(2 / 3):eq((varA / varB).x) + expect(3 / 2):eq((varB / varA).x) + expect(1):eq((varA / varA).x) + expect(1):eq((varB / varB).x) + + expect("23"):eq((varA .. varB).x) + expect("32"):eq((varB .. varA).x) + expect("22"):eq((varA .. varA).x) + expect("33"):eq((varB .. varB).x) + expect("33333"):eq((varB .. varB .. varB .. varB .. varB).x) + + expect(false):eq(varA == varB) + expect(false):eq(varB == varA) + expect(true):eq(varA == varA) + expect(true):eq(varB == varB) + + expect(true):eq(varA < varB) + expect(false):eq(varB < varA) + expect(false):eq(varA < varA) + expect(false):eq(varB < varB) + + expect(true):eq(varA <= varB) + expect(false):eq(varB <= varA) + expect(true):eq(varA <= varA) + expect(true):eq(varB <= varB) + + expect("HELLO"):eq(varA.hello) + varA.hello = "bar" + expect("bar!"):eq(varA.hello) + end) + + + end) + end) +end) diff --git a/src/test/resources/test-rom/spec/lua/timeout_spec.lua b/src/test/resources/test-rom/spec/lua/timeout_spec.lua new file mode 100644 index 000000000..159130d4c --- /dev/null +++ b/src/test/resources/test-rom/spec/lua/timeout_spec.lua @@ -0,0 +1,17 @@ +describe("The VM terminates long running code :slow", function() + it("in loops", function() + expect.error(function() while true do end end) + :str_match("^.+:%d+: Too long without yielding$") + end) + + describe("in string pattern matching", function() + local str, pat = ("a"):rep(1e4), ".-.-.-.-b$" + + it("string.find", function() + expect.error(string.find, str, pat):eq("Too long without yielding") + end) + it("string.match", function() + expect.error(string.match, str, pat):eq("Too long without yielding") + end) + end) +end) diff --git a/src/test/resources/test-rom/spec/lua/vm_spec.lua b/src/test/resources/test-rom/spec/lua/vm_spec.lua new file mode 100644 index 000000000..7f97c6fa1 --- /dev/null +++ b/src/test/resources/test-rom/spec/lua/vm_spec.lua @@ -0,0 +1,9 @@ +describe("The VM", function() + it("allows unpacking a large number of values", function() + -- We don't allow arbitrarily many values, half a meg is probably fine. + -- I don't actually have any numbers on this - maybe we do need more? + local len = 2^19 + local tbl = { (" "):rep(len):byte(1, -1) } + expect(#tbl):eq(len) + end) +end)