Allow and test for custom TestSets in pkg tests

Author: Mal Miller

Project.toml

@@ -1,6 +1,6 @@
 name = "TestReports"
 uuid = "dcd651b4-b50a-5b6b-8f22-87e9f253a252"
-version = "0.5.1"
+version = "0.5.2"
 
 [deps]
 Dates = "ade2ca70-3891-5945-98fb-dc099432e06a"

docs/src/manual.md

@@ -23,7 +23,7 @@ julia> TestReports.test("MyPackage")
     It is intended that it `runtests.jl` will not need to be changed to generate
     a report (unless [properties are being added](#Adding-Properties)).
 
-    This does assume, however, that `DefaultTestSet`s are being used. In the case of
+    This does assume, however, that no custom `TestSet`s are being used. In the case of
     custom `TestSet`s, please see the [discussion](#Custom-TestSet-Types) below.
 
 The typical use in a CI process would be:
@@ -210,9 +210,20 @@ However at a minimum, for a custom `TestSet` type to work with `TestReports` it
 - Push itself onto its parent when finishing, if it is not at the top level
 - Have `description` and `results` fields as per a `DefaultTestSet`
 
-Testsuite properties cannot be added to a `TestSet` that is not a `ReportingTestSet`,
-(i.e. any `TestSet` that has the type specified to be something other than a
-`ReportingTestSet`, or that inherits a specified type from a parent). If no types
-are specified, `TestReports.test` will ensure that all child `TestSet`s inherit
-the `ReportingTestSet` type.
+The following information in a JUnit XML relies on the functionality of `ReportingTestSet`s
+but can be added to your own custom `TestSet` as described in the table.
+
+|Information|Description|
+|---|---|
+| testcase time | This is extracted from a `ReportingResult` by the `TestReports.time_taken` function. For standard `Result`s, rather than `ReportingResult`s, this function returns `Dates.Millisecond(0)`. This function can be extended for other custom `Result` types.|
+| testsuite time| This is extracted from a `TestSet` by the `TestReports.time_taken` function, which can be extended for custom `TestSet`s. If not extended, the `AbstractTestSet` method will be used and the value defaults to `Dates.Millisecond(0)`. |
+| testsuite timestamp| This is extracted from a `TestSet` by the `TestReports.start_time` function, which can be extended for custom `TestSet`s. If not extended, the `AbstractTestSet` method will be used and the value defaults to `Dates.now()`. |
+| testsuite hostname| This is extracted from a `TestSet` by the `TestReports.hostname` function, which can be extended for custom `TestSet`s. If not extended, the `AbstractTestSet` method will be used and the value defaults to `gethostname()`. |
+| testsuite properties| This is extracted from a `TestSet` by the `TestReports.properties` function, which can be extended for custom `TestSet`s. If not extended, the `AbstractTestSet` method will be used and the value defaults to `nothing`. |
+
+For further details on extending these fuctions, see the docstrings in [TestSets](@ref).
+
+The [source code of `TestReports`](https://github.com/JuliaTesting/TestReports.jl/blob/master/src/testsets.jl) can be used as a starting point for including this behaviour in your custom `TestSet`s.
+
+If no `TestSet` types are specified (as per the standard `Test` approach), `TestSet` functionality will ensure that all child `TestSet`s inherit the `ReportingTestSet` type.
 

src/TestReports.jl

@@ -9,6 +9,7 @@ import Test: Result, Fail, Broken, Pass, Error, scrub_backtrace
 
 export ReportingTestSet, any_problems, report, recordproperty
 
+include("v1_compat.jl")
 include("./testsets.jl")
 include("to_xml.jl")
 include("runner.jl")

src/recordproperty.jl

@@ -7,16 +7,17 @@ Adds a property to a testset with `name` and `value` that will in turn be added
 to the `<properties>` node of the corresponding testsuite in the JUnit XML.
 
 Multiple properties can be added to one testset, but if the same property is set on
-both parent and child testsets, the value in the parent testset takes precedence over
-that in the child.
+both parent and child testsets, the value in the child testset takes precedence over
+that in the parent.
 
 The suggested use of this function is to place it inside a testset with unspecified type
 (see Examples). This will ensure that `Pkg.test` is unnaffected, but that the properties
-are added to the report when `TestReports.test` is used. This is because `TestReports`
-wraps package tests in a `ReportingTestSet`, and the function only adds a property when
-it is within a `ReportingTestSet`. 
+are added to the report when `TestReports.test` is used. This is because properties are
+only added when the `Testset` type has a `TestReports.properties` method defined, as does
+the `ReportingTestSet` used by `TestReports`. `TestReports.properties` can be extended
+for custom `TestSet`s.
 
-If a child testset is a `ReportingTestSet` but its parent isn't, the property should
+If a child testset has this method defined but its parent doesn't, the property should
 be in the report when `TestReport.test` is used, assuming that the parent testset
 type doesn't do anything to affect the reporting behaviour. However this is not tested
 functionality.
@@ -37,10 +38,14 @@ using TestReports
     @test 2==2
 end
 ```
+
+See also: [`properties`](@ref)
 """
 function recordproperty(name::String, val)
-    if get_testset() isa ReportingTestSet
-        if haskey(get_testset().properties, name)
+    properties_dict = properties(get_testset())
+    if !isnothing(properties_dict)
+        !isa(properties_dict, AbstractDict) && throw(PkgTestError("TestReports.properties method for custom testset must return a dictionary."))
+        if haskey(properties_dict, name)
             throw(PkgTestError("Property $name already set and can't be set again in the same testset"))
         else
             get_testset().properties["$name"] = val

src/testsets.jl

@@ -1,6 +1,9 @@
 using Dates
 import Test: Result
 
+####################
+# Type Definitions #
+####################
 """
     ReportingResult{T}
 
@@ -17,6 +20,16 @@ end
 Base.:(==)(r1::ReportingResult, r2::ReportingResult) = r1.result == r2.result
 Base.hash(f::ReportingResult, h::UInt) = hash(f.result, h)
 
+"""
+    time_taken(result::ReportingResult)
+    time_taken(result::Result)
+
+For a `ReportingResult`, return the time taken for the test to run.
+For a `Result`, return Dates.Millisecond(0).
+"""
+time_taken(result::ReportingResult) = result.time_taken
+time_taken(result::Result) = Dates.Millisecond(0)
+
 """
     ReportingTestSet
 
@@ -87,8 +100,76 @@ function finish(ts::ReportingTestSet)
     flatten_results!(ts)
 end
 
-#############
+#################################
+# Accessing and setting methods #
+#################################
+"""
+    properties(ts::ReportingTestSet)
+    properties(ts::AbstractTestSet)
+
+Get the properties dictionary of a `ReportingTestSet`, returns
+nothing for an `AbstractTestSet`. Can be extended for custom
+`TestSet`s, and must return either a `Dict` or `nothing`.
+"""
+properties(ts::ReportingTestSet) = ts.properties
+properties(ts::AbstractTestSet) = nothing
+
+"""
+    start_time(ts::ReportingTestSet)
+    start_time(ts::AbstractTestSet)
+
+Get the start time of a `ReportingTestSet`, returns `Dates.now()`
+for an `AbstractTestSet`. Can be extended for custom `TestSet`s,
+must return a `DateTime`.
+"""
+start_time(ts::ReportingTestSet) = ts.start_time
+start_time(ts::AbstractTestSet) = Dates.now()
+
+"""
+    time_taken(ts::ReportingTestSet)
+    time_taken(ts::AbstractTestSet)
+
+Get the time taken of a `ReportingTestSet`, returns `Dates.Millisecond(0)`
+for an `AbstractTestSet`. Can be extended for custom `TestSet`s, must return
+a `Dates.Millisecond`.
+"""
+time_taken(ts::ReportingTestSet) = ts.time_taken
+time_taken(ts::AbstractTestSet) = Dates.Millisecond(0)
+
+"""
+    hostname(ts::ReportingTestSet)
+    hostname(ts::AbstractTestSet)
+
+Get the hostname of a `ReportingTestSet`, returns `gethostname()`
+for an `AbstractTestSet`. Can be extended for custom `TestSet`s,
+must return a `string`.
+"""
+hostname(ts::ReportingTestSet) = ts.hostname
+hostname(ts::AbstractTestSet) = gethostname()
+
+"""
+    set_time_taken!(ts::ReportingTestSet, time_taken)
+    set_time_taken!(ts::AbstractTestSet, time_taken)
+
+Sets the time taken field of a `ReportingTestSet`. This is used when flattening
+`ReportingTestSet`s for report generation and an be extended for custom `TestSet`s.
+"""
+set_time_taken!(ts::ReportingTestSet, time_taken::Millisecond) = ts.time_taken = time_taken
+set_time_taken!(ts::AbstractTestSet, time_taken::Millisecond) = nothing
+
+"""
+    set_start_time!(ts::ReportingTestSet, start_time)
+    set_start_time!(ts::AbstractTestSet, start_time)
+
+Sets the start time field of a `ReportingTestSet`. This is used when flattening
+`ReportingTestSet`s for report generation and an be extended for custom `TestSet`s.
+"""
+set_start_time!(ts::ReportingTestSet, start_time::DateTime) = ts.start_time = start_time
+set_start_time!(ts::AbstractTestSet, start_time::DateTime) = nothing
 
+############
+# Checking #
+############
 """
     any_problems(ts)
 
@@ -104,9 +185,9 @@ any_problems(::Fail) = true
 any_problems(::Broken) = false
 any_problems(::Error) = true
 
-######################################
-# result flattening
-
+#####################
+# Tesult flattening #
+#####################
 
 """
     flatten_results!(ts::AbstractTestSet)
@@ -179,35 +260,34 @@ _flatten_results!(rs::Result) = [rs]
 
 """
     update_testset_properties!(childts::AbstractTestSet, ts::AbstractTestSet)
-    update_testset_properties!(childts::ReportingTestSet, ts::ReportingTestSet)
 
 Adds properties of `ts` to `childts`. If any properties being added already exist in
 `childts`, a warning is displayed and the value in `ts` is overwritten.
 
-If `ts` and\\or `childts` is not a `ReportingTestSet`, this is handled in the
-`AbstractTestSet` method:
-- If `ts` is not a `ReportingTestSet`, it has no properties to add to `childts`
+If the types of `ts` and\\or `childts` do not a method defined for `TestReports.properties`,
+this is handled as follows:
+- If method not defined for `typeof(ts)`, it has no properties to add to `childts`
     and therefore nothing happens.
-- If `childts` is not a `ReportingTestSet` and `ts` has properties, then a warning
+- If method not defined for `typeof(chidlts)` and `ts` has properties, then a warning
     is shown.
+
+See also: [`properties`](@ref)
 """
 function update_testset_properties!(childts::AbstractTestSet, ts::AbstractTestSet)
-    if !isa(childts, ReportingTestSet) && isa(ts, ReportingTestSet) && !isempty(ts.properties)
-        @warn "Properties of testset $(ts.description) can not be added to child testset $(childts.description) as it is not a ReportingTestSet."
-    end
-    # No need to check if childts is ReportingTestSet and ts isn't, as if this is the case
-    # ts has no properties to apply to childts.
-    return childts
-end
-function update_testset_properties!(childts::ReportingTestSet, ts::ReportingTestSet)
-    parent_keys = keys(ts.properties)
-    child_keys = keys(childts.properties)
-    # Loop through keys so that warnings can be issued for any duplicates
-    for key in parent_keys
-        if key in child_keys
-            @warn "Property $key in testest $(ts.description) overwritten by child testset $(childts.description)"
-        else
-            childts.properties[key] = ts.properties[key]
+    if isnothing(properties(childts)) && !isnothing(properties(ts)) && !isempty(properties(ts))
+        @warn "Properties of testset $(ts.description) can not be added to child testset $(childts.description) as it does not have a TestReports.properties method defined."
+        # No need to check if childts is has properties defined and ts doesn't as if this is the case
+        # ts has no properties to add to that of childts.
+    elseif !isnothing(properties(ts))
+        parent_keys = keys(properties(ts))
+        child_keys = keys(properties(childts))
+        # Loop through keys so that warnings can be issued for any duplicates
+        for key in parent_keys
+            if key in child_keys
+                @warn "Property $key in testest $(ts.description) overwritten by child testset $(childts.description)"
+            else
+                properties(childts)[key] = properties(ts)[key]
+            end
         end
     end
     return childts
@@ -231,8 +311,8 @@ function handle_top_level_results!(ts::AbstractTestSet)
         ts.results = AbstractTestSet[]
         ts_nested = ReportingTestSet("Top level tests")
         ts_nested.results = original_results[isa_Result]
-        ts_nested.time_taken = sum(x -> x.time_taken, ts_nested.results)
-        ts_nested.start_time = ts.start_time
+        set_time_taken!(ts_nested, sum(x -> time_taken(x)::Millisecond, ts_nested.results))
+        set_start_time!(ts_nested, start_time(ts)::DateTime)
         push!(ts.results, ts_nested)
         append!(ts.results, original_results[.!isa_Result])
     end