Collapse testsets

.github/workflows/ci.yml

@@ -14,7 +14,7 @@ jobs:
 
     # Change this to add or remove tests
     env:
-      TESTS: all_passing,all_passing_with_debugging,everything_at_once,one_fail,one_passing,truncated_output
+      TESTS: all_passing,all_passing_with_debugging,everything_at_once,one_fail,one_passing,truncated_output,grains,complex-numbers
       JULIA_NUM_THREADS: 2
 
     steps:
@@ -40,8 +40,10 @@ jobs:
           # Create container
           run(`docker create --name jtr-$test jtr-img irrelevant /tmp/ /tmp/`)
 
-          # Copy test file to container
+          # Copy test file(s) to container
           run(`docker cp "test/fixtures/$test/runtests.jl" jtr-$test:/tmp/`)
+          isfile("test/fixtures/$test/$test.jl") &&
+            run(`docker cp "test/fixtures/$test/$test.jl" jtr-$test:/tmp/`)
 
           # Start container to run the test runner
           run(`docker start -a jtr-$test`)
@@ -61,11 +63,23 @@ jobs:
 
         const tests = split(ENV["TESTS"], ",")
 
+        # Shorten the error messages so we don't need to care about specific paths, file numbers, etc.
+        function shorten_messages!(result)
+            if result["status"] == "error" && haskey(result, "message")
+                result["message"] = split(result["message"], '\n')[1]
+            end
+        end
+
         @testset "Compare results.json files" begin
           @testset "$test" for test in tests
               expected = JSON.parsefile(joinpath("test", "fixtures", "$test", "results.json"))
               created = JSON.parsefile(joinpath("results", "$test.json"))
 
+              shorten_messages!(expected)
+              shorten_messages!(created)
+              foreach(shorten_messages!, expected["tests"])
+              foreach(shorten_messages!, created["tests"])
+
               @test expected == created
           end
         end

src/tojson.jl

@@ -1,6 +1,13 @@
 using Test
 using JSON
 
+"""
+Testsets with more than this number of tests will have all of their passing
+tests collapsed into a single report.
+"""
+const TEST_RESULT_COLLAPSE_THRESHOLD = 2
+const MAX_REPORTED_FAILURES_PER_TESTSET = 5
+
 """
     tojson(output::String, ts::ReportingTestSet)
 
@@ -72,69 +79,109 @@ function tojson(output::String, ts::ReportingTestSet)
         end
     end
 
-    # Flag set in walk!(), used for top-level status property
+    # Flag set in push_result!(), used for top-level status property
     any_failed = false
+    # All stdout from the top level test set, used for all tests.
     output = truncate_output(output)
 
     """
-        walk!(prefix, tests, testset)
+        push_result!(tests, result, name)
+
+    Push a test result called `name` to `tests`. Returns the pushed Dict or `nothing` if the test was not pushed.
+    """
+    function push_result!(tests, result::Test.Result, name)
+        status = nothing
+        message = nothing
+        test_code = nothing
+
+        if result isa Test.Pass
+            status = "pass"
+            test_code = "@test $(result.orig_expr)"
+        elseif result isa Test.Fail
+            status = "fail"
+            message = string(result)
+            test_code = "@test $(result.orig_expr)"
+        elseif result isa Test.LogTestFailure
+            status = "fail"
+            message = string(result)
+            test_code = "@test_logs $(join(result.patterns, ' ')) $(result.orig_expr)"
+        elseif result isa Test.Error
+            status = "error"
+            message = result.backtrace
+            test_code = "@test " * result.orig_expr
+        elseif result isa Test.Broken
+            if result.test_type === :skipped
+                return nothing
+            end
+            # TODO: In the future we might have a new `status = skip`
+            message = string(result)
+            test_code = result.test_type === :skipped ? "@test_skip " : "@test_broken "
+            test_code *= string(result.orig_expr)
+        else
+            error("Unknown testset.results item: $result")
+        end
+
+        any_failed = any_failed || status in ("fail", "error")
+
+        return push!(tests, Dict(filter( ((k, v),) -> !isnothing(v), (
+            "name" => name,
+            "status" => status,
+            "message" => message,
+            "test_code" => test_code,
+            "output" => output,
+        ))))
+    end
+
+    """
+        walk!(tests, prefix, testset)
 
     Walk the tree of testsets, pushing Dicts to `tests` describing each test
     result. Returns nothing.
     """
     function walk!(tests, prefix, testset)
         name = isempty(prefix) ? testset.description : "$prefix » $(testset.description)"
 
-        # Should we number the tests?
-        number_tests = count(x -> x isa Test.Result, testset.results) > 1
+        num_results = count(x -> x isa Test.Result, testset.results)
 
-        for (n, result) in enumerate(testset.results)
-            status = nothing
-            message = nothing
-            test_code = nothing
-
-            if result isa Test.Pass
-                status = "pass"
-                message = nothing
-            elseif result isa Test.Fail
-                status = "fail"
-                message = string(result)
-                test_code = "@test " * result.orig_expr
-            elseif result isa Test.LogTestFailure
-                status = "fail"
-                message = string(result)
-                test_code = "@test_logs $(join(result.patterns, ' ')) $(result.orig_expr)"
-            elseif result isa Test.Error
-                status = "error"
-                message = result.backtrace
-                test_code = "@test " * result.orig_expr
-            elseif result isa Test.Broken
-                if result.test_type === :skipped
-                    continue
-                end
-                # TODO: In the future we might have a new `status = skip`
-                message = string(result)
-                test_code = result.test_type === :skipped ? "@test_skip " : "@test_broken "
-                test_code *= string(result.orig_expr)
-            elseif result isa Test.AbstractTestSet
-                # Descend into nested test sets
-                child_has_failed = walk!(tests, name, result)
-                continue
+        function test_name(result, idx)
+            if name == "" # Tests that aren't in a testset
+                string(result.orig_expr)
             else
-                error("Unknown testset.results item: $result")
+                # If there's more than one test in the testset, distinguish
+                # them with numbers.
+                num_results > 1 ? "$name.$idx" : name
             end
+        end
 
-            any_failed = any_failed || status in ("fail", "error")
-
-            push!(tests, Dict(filter( ((k, v),) -> !isnothing(v), (
-                "name" => number_tests ? "$name.$n" : name,
-                "status" => status,
-                "message" => message,
-                "test_code" => test_code,
-                "output" => output,
-            ))))
+        # Should we collapse all passing tests into one report?
+        passing_tests = filter(x -> x isa Test.Pass, testset.results)
+        num_passing = length(passing_tests)
+        collapse_passing_tests = num_results >= TEST_RESULT_COLLAPSE_THRESHOLD && num_passing > 1 && name != ""
+
+        if collapse_passing_tests
+            test_code = join(("@test $(r.orig_expr)" for r in passing_tests), '\n')
+            collapsed_name = num_passing == num_results ? name : "$name » $num_passing tests"
+            push!(tests, Dict(
+                "name" => collapsed_name,
+                "status" => "pass",
+                "test_code" => test_code
+            ))
         end
 
+        # Push up to MAX_REPORTED_FAILURES_PER_TESTSET failing test results.
+        # Also recurse into any test sets and report passing test results if
+        # they weren't collapsed.
+        num_reported_failures = 0
+        for (n, result) in enumerate(testset.results)
+            if result isa Test.AbstractTestSet
+                walk!(tests, name, result)
+            elseif result isa Test.Pass
+                collapse_passing_tests || push_result!(tests, result, test_name(result, n))
+            else
+                num_reported_failures >= MAX_REPORTED_FAILURES_PER_TESTSET && continue
+                num_reported_failures += !isnothing(push_result!(tests, result, test_name(result, n)))
+            end
+        end
         return nothing
     end
 

test/fixtures/all_passing/results.json

@@ -4,14 +4,17 @@
     "tests": [
         {
             "name": "first test",
+            "test_code": "@test x == 1",
             "status": "pass"
         },
         {
             "name": "second test",
+            "test_code": "@test 2 == 2",
             "status": "pass"
         },
         {
             "name": "third test",
+            "test_code": "@test 3 == 3",
             "status": "pass"
         }
     ]

test/fixtures/all_passing_with_debugging/results.json

@@ -4,11 +4,13 @@
     "tests": [
         {
             "name": "first test",
+            "test_code": "@test x == 1",
             "status": "pass",
             "output": "x = 1\nI'M HERE!\n"
         },
         {
             "name": "second test",
+            "test_code": "@test 2 == 2",
             "status": "pass",
             "output": "x = 1\nI'M HERE!\n"
         }

test/fixtures/complex-numbers/complex-numbers.jl

@@ -0,0 +1,59 @@
+import Base: ==, +, -, *, /, abs, conj, exp, imag, real, zero, promote_rule
+
+struct ComplexNumber{T <: Real} <: Number
+    a::T
+    b::T
+end
+
+function ComplexNumber(a::T, b::U) where {T <: Real, U <: Real}
+    V = promote_type(T, U)
+    ComplexNumber{V}(V(a), V(b))
+end
+
+# This makes isapprox work and is needed for Bonus 2.
+ComplexNumber{T}(a::Real) where T = ComplexNumber{T}(T(a), zero(T))
+
+# Convert ComplexNumber{U} to ComplexNumber{T}, required for Bonus 2.
+ComplexNumber{T}(a::ComplexNumber) where T = ComplexNumber{T}(T(a.a), T(a.b))
+
+# You can make isapprox work without defining the constructor above if you
+# define zero and real
+zero(::Type{ComplexNumber{T}}) where T = ComplexNumber(zero(T), zero(T))
+zero(x::ComplexNumber{T}) where T = ComplexNumber(zero(T), zero(T))
+
+real(::Type{ComplexNumber{T}}) where T = T
+real(x::ComplexNumber) = x.a
+
+imag(::Type{ComplexNumber{T}}) where T = T
+imag(x::ComplexNumber) = x.b
+
+# Default structural equality method uses ===, but we will often contain
+# floats, which must be compared with ==
+==(x::ComplexNumber, y::ComplexNumber) = x.a == y.a && x.b == y.b
+
+conj(x::ComplexNumber) = ComplexNumber(x.a, -x.b)
+abs(x::ComplexNumber) = sqrt(x.a * x.a + x.b * x.b)
+
+-(x::ComplexNumber) = ComplexNumber(-x.a, -x.b)
++(x::ComplexNumber, y::ComplexNumber) = ComplexNumber(x.a + y.a, x.b + y.b)
+-(x::ComplexNumber, y::ComplexNumber) = x + -y
+
+*(x::ComplexNumber, y::ComplexNumber) = ComplexNumber(x.a * y.a - x.b * y.b,
+                                                      x.b * y.a + x.a * y.b)
+
+/(x::ComplexNumber, y::ComplexNumber) = ComplexNumber((x.a * y.a + x.b * y.b) / (y.a^2 + y.b^2),
+                                                      (x.b * y.a - x.a * y.b) / (y.a^2 + y.b^2))
+
+# Bonus 1: exp
+
+# Promotion rule makes mixed Real/Complex mathematical operations work
+
+# Commenting this out makes some of the tests fail, which is useful for testing the test runner :)
+# promote_rule(::Type{T}, ::Type{ComplexNumber{U}}) where {T <: Real, U} = ComplexNumber{promote_type(T, U)}
+exp(x::ComplexNumber) = ℯ^x.a * ComplexNumber(cos(x.b), sin(x.b))
+
+# Bonus 2: jm unit
+
+# Better to use false here because we need that strong multiplicative zero
+# to make `0 + NaN * jm === ComplexNumber(0, NaN)`
+const jm = ComplexNumber(false, true)