Make ReportingTestSet behaviour consistent

Author: Mal Miller

Project.toml

@@ -1,6 +1,6 @@
 name = "TestReports"
 uuid = "dcd651b4-b50a-5b6b-8f22-87e9f253a252"
-version = "0.3.3"
+version = "0.4.0"
 
 [deps]
 EzXML = "8f5d6c58-4d21-5cfd-889c-e3ad7ee6a615"

bin/reporttests.jl

@@ -11,8 +11,6 @@ ts = @testset ReportingTestSet "" begin
     include($(repr(testfilename)))
 end
 
-display_reporting_testset(ts)
-
 open("testlog.xml","w") do fh
     print(fh, report(ts))
 end

src/TestReports.jl

@@ -7,8 +7,7 @@ using EzXML
 import Test: AbstractTestSet, DefaultTestSet, record, finish, get_testset_depth, get_testset
 import Test: Result, Fail, Broken, Pass, Error, scrub_backtrace
 
-export ReportingTestSet, any_problems, report, display_reporting_testset,
-    recordproperty
+export ReportingTestSet, any_problems, report, recordproperty
 
 include("./testsets.jl")
 include("to_xml.jl")

src/runner.jl

@@ -16,8 +16,6 @@ function gen_runner_code(testfilename, logfilename, testreportsdir, test_args)
         include($(repr(testfilename)))
     end
 
-    display_reporting_testset(ts)
-
     write($(repr(logfilename)), report(ts))
     any_problems(ts) && exit(TestReports.TESTS_FAILED)
     """

src/testsets.jl

@@ -1,4 +1,25 @@
+"""
+    ReportingTestSet
+
+Custom `AbstractTestSet` type designed to be used by `TestReports.jl` for
+creation of JUnit XMLs.
+
+Does not throw an error when a test fails or has an error. Upon `finish`ing,
+a `ReportingTestSet` will display the default test output, and then flatten
+to a structure that is suitable for report generation.
+
+It is designed to be wrapped around a package's `runtests.jl` file and this
+is assumed when both the test results are displayed and when the `TestSet` is
+flatted upon `finish`. See `bin/reporttests.jl` for an example of this use.
+`ReportingTestSet`s are not designed to be used directly in a package's tests,
+and this is not recommended or supported.
 
+A `ReportingTestSet` has the `description` and `results` fields as per a
+`DefaultTestSet`, and has an additional `properties` field which is used
+to record properties to be inserted into the report.
+
+See also: [`flatten_results!`](@ref), [`recordproperty`](@ref), [`report`](@ref)
+"""
 mutable struct ReportingTestSet <: AbstractTestSet
     description::String
     results::Vector
@@ -19,6 +40,9 @@ function finish(ts::ReportingTestSet)
         return ts
     end
 
+    # Display before flattening to match Pkg.test output
+    display_reporting_testset(ts)
+
     # We are the top level, lets do this
     flatten_results!(ts)
 end

src/to_xml.jl

@@ -120,6 +120,7 @@ That is, the results of the top level `TestSet` must all be `AbstractTestSet`s,
 and the results of those `TestSet`s must all be `Results`.
 """
 function report(ts::AbstractTestSet)
+    check_ts_structure(ts)
     total_ntests = 0
     total_nfails = 0
     total_nerrors = 0
@@ -142,6 +143,20 @@ function report(ts::AbstractTestSet)
     xdoc
 end
 
+"""
+    check_ts_structure(ts::AbstractTestSet)
+
+Throws an exception if `ts` does not have the right structure for `report`.
+
+See also: [`report`](@ref)
+"""
+function check_ts_structure(ts::AbstractTestSet)
+    !all(isa.(ts.results, AbstractTestSet)) && throw(ArgumentError("Results of ts must all be AbstractTestSets. See documentation for `report`."))
+    for result in ts.results
+        !all(isa.(result.results, Result)) && throw(ArgumentError("Results of each AbstractTestSet in ts.results must all be Results. See documentation for `report`."))
+    end
+end
+
 """
     to_xml(ts::AbstractTestSet)
 

test/recordproperty.jl

@@ -87,7 +87,7 @@ end
             TestReports.test(pkg)
         end
         logfile = joinpath(@__DIR__, "testlog.xml")
-        @test_reference "references/test_with_properties.txt" open(f->read(f, String), logfile) |> clean_report
+        @test_reference "references/test_with_properties.txt" open(f->read(f, String), logfile) |> clean_output
     end
 
     # Test for warning when ID set twice

test/runtests.jl

@@ -12,16 +12,18 @@ include("testsets.jl")
 include("recordproperty.jl")
 
 @testset "SingleNest" begin
-    @test_reference "references/singlenest.txt" read(`$(Base.julia_cmd()) -e "using Test; using TestReports; (@testset ReportingTestSet \"blah\" begin @testset \"a\" begin @test 1 ==1 end end) |> report |> print"`, String) |> clean_report
+    @test_reference "references/singlenest.txt" read(`$(Base.julia_cmd()) -e "using Test; using TestReports; (@testset ReportingTestSet \"blah\" begin @testset \"a\" begin @test 1 ==1 end end) |> report |> print"`, String) |>  clean_output
 end
 
 @testset "Complex Example" begin
-    @test_reference "references/complexexample.txt" read(`$(Base.julia_cmd()) $(@__DIR__)/example.jl`, String) |> clean_report
+    @test_reference "references/complexexample.txt" read(`$(Base.julia_cmd()) $(@__DIR__)/example.jl`, String) |> clean_output
 end
 
 @testset "Chained failing test - Issue #25" begin
     ts = @testset ReportingTestSet begin
+        ts = @testset ReportingTestSet begin
             @test 1==1 && 1==0
+        end
     end
     @test report(ts) isa Any  # Would fail before #25
 end
@@ -180,5 +182,32 @@ end
     rm(new_path, recursive=true)
 end
 
+@testset "record - check_ts_structure" begin
+    # No top level testset
+    ts = @testset NoFlattenReportingTestSet begin
+        @test true
+    end
+    @test_throws ArgumentError TestReports.check_ts_structure(ts)
+
+    # Not flattened
+    ts = @testset NoFlattenReportingTestSet begin
+        @testset NoFlattenReportingTestSet begin
+            @test true
+            @testset NoFlattenReportingTestSet begin
+                @test true
+            end
+        end
+    end
+    @test_throws ArgumentError TestReports.check_ts_structure(ts)
+
+    # Correct structure
+    ts = @testset NoFlattenReportingTestSet begin
+        @testset NoFlattenReportingTestSet begin
+            @test true
+        end
+    end
+    @test TestReports.check_ts_structure(ts) isa Any # Doesn't error
+end
+
 # clean up locally cached registry
 rm(joinpath(@__DIR__, "registries"); force=true, recursive=true)

test/testsets.jl

@@ -1,24 +1,7 @@
 using Test
-import Test: finish, record, AbstractTestSet, Result, get_testset_depth, get_testset,
-    Pass, Fail, Broken, Error
+import Test: AbstractTestSet, Result, Pass, Fail, Broken, Error
 using TestReports
 
-mutable struct NoFlattenReportingTestSet <: AbstractTestSet
-    description::AbstractString
-    results::Vector
-end
-NoFlattenReportingTestSet(desc) = NoFlattenReportingTestSet(desc, [])
-record(ts::NoFlattenReportingTestSet, t) = (push!(ts.results, t); t)
-function finish(ts::NoFlattenReportingTestSet)
-    if get_testset_depth() != 0
-        # Attach this test set to the parent test set
-        parent_ts = get_testset()
-        record(parent_ts, ts)
-        return ts
-    end
-    return ts
-end
-
 @testset "handle_top_level_results!" begin
     # Simple top level resuls
     ts = @testset NoFlattenReportingTestSet "" begin

test/utils.jl

@@ -1,4 +1,5 @@
 using Pkg, Test, LibGit2
+import Test: finish, record, AbstractTestSet, get_testset_depth, get_testset
 
 # Strip the filenames from the string, so that the reference strings work on different computers
 strip_filepaths(str) = replace(str, r" at .*\d+$"m => "")
@@ -12,7 +13,10 @@ replace_Int32s(str) = replace(str, "Int32" => "Int64")
 # remove stacktraces so reference strings work for different Julia versions
 remove_stacktraces(str) = replace(str, r"(Stacktrace:)[^<]*" => "")
 
-const clean_report = replace_Int32s ∘ replace_windows_filepaths ∘ strip_filepaths ∘ remove_stacktraces
+# remove test output - remove everything before "<?xml version"
+remove_test_output(str) = replace(str, r"^[\S\s]*(?=(<\?xml version))" => "")
+
+const clean_output = replace_Int32s ∘ replace_windows_filepaths ∘ strip_filepaths ∘ remove_stacktraces ∘ remove_test_output
 
 """
 `copy_test_package` copied from [`Pkg.jl/test/utils.jl`](https://github.com/JuliaLang/Pkg.jl/blob/v1.4.2/test/utils.jl#L209).
@@ -146,3 +150,20 @@ function test_active_package_expected_fail(pkg::String)
         @test_throws TestReports.PkgTestError TestReports.test(pkg)
     end
 end
+
+# Test TestSets
+mutable struct NoFlattenReportingTestSet <: AbstractTestSet
+    description::AbstractString
+    results::Vector
+end
+NoFlattenReportingTestSet(desc) = NoFlattenReportingTestSet(desc, [])
+record(ts::NoFlattenReportingTestSet, t) = (push!(ts.results, t); t)
+function finish(ts::NoFlattenReportingTestSet)
+    if get_testset_depth() != 0
+        # Attach this test set to the parent test set
+        parent_ts = get_testset()
+        record(parent_ts, ts)
+        return ts
+    end
+    return ts
+end