From 6b3773a8624d9efcdec504bc75a9e5e355ccd6c5 Mon Sep 17 00:00:00 2001 From: SquidDev Date: Mon, 11 May 2020 15:41:39 +0100 Subject: [PATCH] Run tests with coverage - Use jacoco for Java-side coverage. Our Java coverage is /terrible (~10%), as we only really test the core libraries. Still a good thing to track for regressions though. - mcfly now tracks Lua side coverage. This works in several stages: - Replace loadfile to include the whole path - Add a debug hook which just tracks filename->(lines->count). This is then submitted to the Java test runner. - On test completion, we emit a luacov.report.out file. As the debug hook is inserted by mcfly, this does not include any computer startup (such as loading apis, or the root of bios.lua), despite they're executed. This would be possible to do (for instance, inject a custom header into bios.lua). However, we're not actually testing any of the behaviour of startup (aside from "does it not crash"), so I'm not sure whether to include it or not. Something I'll most likely re-evaluate. --- .github/workflows/main-ci.yml | 3 + build.gradle | 10 ++ .../core/ComputerTestDelegate.java | 24 ++- .../computercraft/core/LuaCoverage.java | 158 ++++++++++++++++++ src/test/resources/test-rom/mcfly.lua | 50 ++++++ .../test-rom/spec/apis/peripheral_spec.lua | 2 +- .../test-rom/spec/apis/textutils_spec.lua | 2 +- .../resources/test-rom/spec/base_spec.lua | 2 + .../test-rom/spec/modules/cc/expect_spec.lua | 2 +- 9 files changed, 249 insertions(+), 4 deletions(-) create mode 100644 src/test/java/dan200/computercraft/core/LuaCoverage.java diff --git a/.github/workflows/main-ci.yml b/.github/workflows/main-ci.yml index 0aee2ef4f..615f24f30 100644 --- a/.github/workflows/main-ci.yml +++ b/.github/workflows/main-ci.yml @@ -32,6 +32,9 @@ jobs: name: CC-Tweaked path: build/libs + - name: Upload Coverage + run: bash <(curl -s https://codecov.io/bash) + lint-lua: name: Lint Lua runs-on: ubuntu-latest diff --git a/build.gradle b/build.gradle index df2375cc5..2c93873ee 100644 --- a/build.gradle +++ b/build.gradle @@ -18,6 +18,7 @@ plugins { id "checkstyle" + id "jacoco" id "com.github.hierynomus.license" version "0.15.0" id "com.matthewprenger.cursegradle" version "1.3.0" id "com.github.breadmoirai.github-release" version "2.2.4" @@ -251,6 +252,15 @@ task compressJson(dependsOn: extractAnnotationsJar) { } } +jacocoTestReport { + reports { + xml.enabled true + html.enabled true + } +} + +check.dependsOn jacocoTestReport + license { mapping("java", "SLASHSTAR_STYLE") strictCheck true diff --git a/src/test/java/dan200/computercraft/core/ComputerTestDelegate.java b/src/test/java/dan200/computercraft/core/ComputerTestDelegate.java index a3410f04e..ee1d5ebaa 100644 --- a/src/test/java/dan200/computercraft/core/ComputerTestDelegate.java +++ b/src/test/java/dan200/computercraft/core/ComputerTestDelegate.java @@ -30,12 +30,14 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; +import java.io.BufferedWriter; import java.io.File; import java.io.IOException; import java.io.Writer; import java.nio.channels.Channels; import java.nio.channels.WritableByteChannel; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import java.util.*; import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.Condition; @@ -58,6 +60,8 @@ */ public class ComputerTestDelegate { + private static final File REPORT_PATH = new File( "test-files/luacov.report.out" ); + private static final Logger LOG = LogManager.getLogger( ComputerTestDelegate.class ); private static final long TICK_TIME = TimeUnit.MILLISECONDS.toNanos( 50 ); @@ -77,6 +81,7 @@ public class ComputerTestDelegate private final Condition hasFinished = lock.newCondition(); private boolean finished = false; + private Map> finishedWith; @BeforeEach public void before() throws IOException @@ -84,6 +89,8 @@ public void before() throws IOException ComputerCraft.logPeripheralErrors = true; ComputerCraft.log = LogManager.getLogger( ComputerCraft.MOD_ID ); + if( REPORT_PATH.delete() ) ComputerCraft.log.info( "Deleted previous coverage report." ); + Terminal term = new Terminal( 78, 20 ); IWritableMount mount = new FileMount( new File( "test-files/mount" ), 10_000_000 ); @@ -265,6 +272,13 @@ else if( !computer.isOn() ) try { finished = true; + if( arguments.length > 0 ) + { + @SuppressWarnings( "unchecked" ) + Map> finished = (Map>) arguments[0]; + finishedWith = finished; + } + hasFinished.signal(); } finally @@ -282,7 +296,7 @@ else if( !computer.isOn() ) } @AfterEach - public void after() throws InterruptedException + public void after() throws InterruptedException, IOException { try { @@ -317,6 +331,14 @@ public void after() throws InterruptedException // And shutdown computer.shutdown(); } + + if( finishedWith != null ) + { + try( BufferedWriter writer = Files.newBufferedWriter( REPORT_PATH.toPath() ) ) + { + new LuaCoverage( finishedWith ).write( writer ); + } + } } @TestFactory diff --git a/src/test/java/dan200/computercraft/core/LuaCoverage.java b/src/test/java/dan200/computercraft/core/LuaCoverage.java new file mode 100644 index 000000000..e6e61e8f2 --- /dev/null +++ b/src/test/java/dan200/computercraft/core/LuaCoverage.java @@ -0,0 +1,158 @@ +/* + * This file is part of ComputerCraft - http://www.computercraft.info + * Copyright Daniel Ratcliffe, 2011-2020. Do not distribute without permission. + * Send enquiries to dratcliffe@gmail.com + */ + +package dan200.computercraft.core; + +import com.google.common.base.Strings; +import dan200.computercraft.ComputerCraft; +import it.unimi.dsi.fastutil.ints.IntOpenHashSet; +import it.unimi.dsi.fastutil.ints.IntSet; +import org.squiddev.cobalt.Prototype; +import org.squiddev.cobalt.compiler.CompileException; +import org.squiddev.cobalt.compiler.LuaC; + +import java.io.*; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.*; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +class LuaCoverage +{ + private static final Path ROOT = new File( "src/main/resources/assets/computercraft/lua" ).toPath(); + private static final Path BIOS = ROOT.resolve( "bios.lua" ); + private static final Path APIS = ROOT.resolve( "rom/apis" ); + private static final Path SHELL = ROOT.resolve( "rom/programs/shell.lua" ); + private static final Path MULTISHELL = ROOT.resolve( "rom/programs/advanced/multishell.lua" ); + private static final Path TREASURE = ROOT.resolve( "treasure" ); + + private final Map> coverage; + private final String blank; + private final String zero; + private final String countFormat; + + LuaCoverage( Map> coverage ) + { + this.coverage = coverage; + + int max = (int) coverage.values().stream() + .flatMapToDouble( x -> x.values().stream().mapToDouble( y -> y ) ) + .max().orElse( 0 ); + int maxLen = Math.max( 1, (int) Math.ceil( Math.log10( max ) ) ); + blank = Strings.repeat( " ", maxLen + 1 ); + zero = Strings.repeat( "*", maxLen ) + "0"; + countFormat = "%" + (maxLen + 1) + "d"; + } + + void write( Writer out ) throws IOException + { + Files.find( ROOT, Integer.MAX_VALUE, ( path, attr ) -> attr.isRegularFile() && !path.startsWith( TREASURE ) ).forEach( path -> { + Path relative = ROOT.relativize( path ); + String full = relative.toString().replace( '\\', '/' ); + if( !full.endsWith( ".lua" ) ) return; + + Map files = Stream.of( + coverage.remove( "/" + full ), + path.equals( BIOS ) ? coverage.remove( "bios.lua" ) : null, + path.equals( SHELL ) ? coverage.remove( "shell.lua" ) : null, + path.equals( MULTISHELL ) ? coverage.remove( "multishell.lua" ) : null, + path.startsWith( APIS ) ? coverage.remove( path.getFileName().toString() ) : null + ) + .filter( Objects::nonNull ) + .flatMap( x -> x.entrySet().stream() ) + .collect( Collectors.toMap( Map.Entry::getKey, Map.Entry::getValue, Double::sum ) ); + + try + { + writeCoverageFor( out, path, files ); + } + catch( IOException e ) + { + throw new UncheckedIOException( e ); + } + } ); + + for( String filename : coverage.keySet() ) + { + if( filename.startsWith( "/test-rom/" ) ) continue; + ComputerCraft.log.warn( "Unknown file {}", filename ); + } + } + + private void writeCoverageFor( Writer out, Path fullName, Map visitedLines ) throws IOException + { + if( !Files.exists( fullName ) ) + { + ComputerCraft.log.error( "Cannot locate file {}", fullName ); + return; + } + + IntSet activeLines = getActiveLines( fullName.toFile() ); + + out.write( "==============================================================================\n" ); + out.write( fullName.toString().replace( '\\', '/' ) ); + out.write( "\n" ); + out.write( "==============================================================================\n" ); + + try( BufferedReader reader = Files.newBufferedReader( fullName ) ) + { + String line; + int lineNo = 0; + while( (line = reader.readLine()) != null ) + { + lineNo++; + Double count = visitedLines.get( (double) lineNo ); + if( count != null ) + { + out.write( String.format( countFormat, count.intValue() ) ); + } + else if( activeLines.contains( lineNo ) ) + { + out.write( zero ); + } + else + { + out.write( blank ); + } + + out.write( ' ' ); + out.write( line ); + out.write( "\n" ); + } + } + } + + private static IntSet getActiveLines( File file ) throws IOException + { + IntSet activeLines = new IntOpenHashSet(); + try( InputStream stream = new FileInputStream( file ) ) + { + Prototype proto = LuaC.compile( stream, "@" + file.getPath() ); + Queue queue = new ArrayDeque<>(); + queue.add( proto ); + + while( (proto = queue.poll()) != null ) + { + int[] lines = proto.lineinfo; + if( lines != null ) + { + for( int line : lines ) + { + activeLines.add( line ); + } + } + if( proto.p != null ) Collections.addAll( queue, proto.p ); + } + } + catch( CompileException e ) + { + throw new IllegalStateException( "Cannot compile", e ); + } + + return activeLines; + } +} diff --git a/src/test/resources/test-rom/mcfly.lua b/src/test/resources/test-rom/mcfly.lua index 5081447a6..ddd4f46b3 100644 --- a/src/test/resources/test-rom/mcfly.lua +++ b/src/test/resources/test-rom/mcfly.lua @@ -483,6 +483,49 @@ local function pending(name) test_stack.n = n - 1 end +local native_co_create, native_loadfile = coroutine.create, loadfile +local line_counts = {} +if cct_test then + local string_sub, debug_getinfo = string.sub, debug.getinfo + local function debug_hook(_, line_nr) + local name = debug_getinfo(2, "S").source + if string_sub(name, 1, 1) ~= "@" then return end + name = string_sub(name, 2) + + local file = line_counts[name] + if not file then file = {} line_counts[name] = file end + file[line_nr] = (file[line_nr] or 0) + 1 + end + + coroutine.create = function(...) + local co = native_co_create(...) + debug.sethook(co, debug_hook, "l") + return co + end + + local expect = require "cc.expect".expect + _G.native_loadfile = native_loadfile + _G.loadfile = function(filename, mode, env) + -- Support the previous `loadfile(filename, env)` form instead. + if type(mode) == "table" and env == nil then + mode, env = nil, mode + end + + expect(1, filename, "string") + expect(2, mode, "string", "nil") + expect(3, env, "table", "nil") + + local file = fs.open(filename, "r") + if not file then return nil, "File not found" end + + local func, err = load(file.readAll(), "@/" .. fs.combine(filename, ""), mode, env) + file.close() + return func, err + end + + debug.sethook(debug_hook, "l") +end + local arg = ... if arg == "--help" or arg == "-h" then io.write("Usage: mcfly [DIR]\n") @@ -648,4 +691,11 @@ if test_status.pending > 0 then end term.setTextColour(colours.white) io.write(info .. "\n") + +-- Restore hook stubs +debug.sethook(nil, "l") +coroutine.create = native_co_create +_G.loadfile = native_loadfile + +if cct_test then cct_test.finish(line_counts) end if howlci then howlci.log("debug", info) sleep(3) end diff --git a/src/test/resources/test-rom/spec/apis/peripheral_spec.lua b/src/test/resources/test-rom/spec/apis/peripheral_spec.lua index 1b11c6552..4fea6ddf0 100644 --- a/src/test/resources/test-rom/spec/apis/peripheral_spec.lua +++ b/src/test/resources/test-rom/spec/apis/peripheral_spec.lua @@ -51,7 +51,7 @@ describe("The peripheral library", function() it_modem("has the correct error location", function() expect.error(function() peripheral.call("top", "isOpen", false) end) - :str_match("^peripheral_spec.lua:%d+: bad argument #1 %(number expected, got boolean%)$") + :str_match("^[^:]+:%d+: bad argument #1 %(number expected, got boolean%)$") end) end) diff --git a/src/test/resources/test-rom/spec/apis/textutils_spec.lua b/src/test/resources/test-rom/spec/apis/textutils_spec.lua index bdef2dbd2..94e243afc 100644 --- a/src/test/resources/test-rom/spec/apis/textutils_spec.lua +++ b/src/test/resources/test-rom/spec/apis/textutils_spec.lua @@ -49,7 +49,7 @@ describe("The textutils library", function() describe("textutils.empty_json_array", function() it("is immutable", function() expect.error(function() textutils.empty_json_array[1] = true end) - :eq("textutils_spec.lua:51: attempt to mutate textutils.empty_json_array") + :str_match("^[^:]+:51: attempt to mutate textutils.empty_json_array$") end) end) diff --git a/src/test/resources/test-rom/spec/base_spec.lua b/src/test/resources/test-rom/spec/base_spec.lua index 2e557e132..85bbf3f98 100644 --- a/src/test/resources/test-rom/spec/base_spec.lua +++ b/src/test/resources/test-rom/spec/base_spec.lua @@ -28,6 +28,8 @@ describe("The Lua base library", function() end) describe("loadfile", function() + local loadfile = _G.native_loadfile or loadfile + local function make_file() local tmp = fs.open("test-files/out.lua", "w") tmp.write("return _ENV") diff --git a/src/test/resources/test-rom/spec/modules/cc/expect_spec.lua b/src/test/resources/test-rom/spec/modules/cc/expect_spec.lua index 125e43640..1a30a3db7 100644 --- a/src/test/resources/test-rom/spec/modules/cc/expect_spec.lua +++ b/src/test/resources/test-rom/spec/modules/cc/expect_spec.lua @@ -27,7 +27,7 @@ describe("cc.expect", function() worker() end - expect.error(trampoline):eq("expect_spec.lua:27: bad argument #1 to 'worker' (expected string, got nil)") + expect.error(trampoline):str_match("^[^:]*expect_spec.lua:27: bad argument #1 to 'worker' %(expected string, got nil%)$") end) end)