Replace relpath with cheaper function (#174)

* Replace `relpath` with cheaper function

Since we call `relpath` for every directory and file
in the directory tree before any tests start, we want
this to be as cheap as possible (to minimise latency).
Hopefully this new function is valid on Windows, but
I've no machine to test that :/

* Make type-stable

* Change fields to SubString to save an alloc

Author: nickrobinson251

src/ReTestItems.jl

@@ -633,6 +633,22 @@ function is_testsetup_file(filepath)
     )
 end
 
+# Like `relpath` but assumes `path` is nested under `startdir`, else just returns `path`.
+# Always returns a `SubString` to be type-stable.
+function nestedrelpath(path::T, startdir::AbstractString) where {T <: AbstractString}
+    path == startdir && return SubString{T}(".")
+    relp = chopprefix(path, startdir)
+    relp == path && return relp
+    sep = Base.Filesystem.path_separator
+    if endswith(startdir, sep)
+        return relp
+    elseif startswith(relp, sep)
+        return chopprefix(relp, sep)
+    else # `startdir` was a prefix of `path` but not a directory
+        return SubString{T}(path)
+    end
+end
+
 # is `dir` the root of a subproject inside the current project?
 function _is_subproject(dir, current_projectfile)
     projectfile = _project_file(dir)
@@ -642,7 +658,7 @@ function _is_subproject(dir, current_projectfile)
     projectfile == current_projectfile && return false
     # a `test/Project.toml` is special and doesn't indicate a subproject
     current_project_dir = dirname(current_projectfile)
-    rel_projectfile = relpath(projectfile, current_project_dir)
+    rel_projectfile = nestedrelpath(projectfile, current_project_dir)
     rel_projectfile == joinpath("test", "Project.toml") && return false
     return true
 end
@@ -675,7 +691,7 @@ function include_testfiles!(project_name, projectfile, paths, ti_filter::TestIte
             subproject_root = root
             continue
         end
-        rpath = relpath(root, project_root)
+        rpath = nestedrelpath(root, project_root)
         startswith(rpath, hidden_re) && continue # skip hidden directories
         dir_node = DirNode(rpath; report, verbose=verbose_results)
         dir_nodes[rpath] = dir_node
@@ -693,7 +709,7 @@ function include_testfiles!(project_name, projectfile, paths, ti_filter::TestIte
             if !(is_testsetup_file(filepath) || (is_test_file(filepath) && is_requested(filepath, paths)))
                 continue
             end
-            fpath = relpath(filepath, project_root)
+            fpath = nestedrelpath(filepath, project_root)
             file_node = FileNode(fpath, ti_filter; report, verbose=verbose_results)
             testitem_names = Set{String}() # to enforce that names in the same file are unique
             push!(dir_node, file_node)
@@ -745,7 +761,7 @@ function _throw_duplicate_ids(testitems)
     seen = Dict{String,String}()
     for ti in testitems
         id = ti.id
-        source = string(relpath(ti.file, ti.project_root), ":", ti.line)
+        source = string(nestedrelpath(ti.file, ti.project_root), ":", ti.line)
         name = string(repr(ti.name), " at ", source)
         if haskey(seen, id)
             name1 = seen[id]

src/junit_xml.jl

@@ -98,15 +98,15 @@ mutable struct JUnitTestSuite  # File
     counts::JUnitCounts
     testcases::Vector{JUnitTestCase}
 end
-JUnitTestSuite(name::String) = JUnitTestSuite(name, JUnitCounts(), JUnitTestCase[])
+JUnitTestSuite(name::AbstractString) = JUnitTestSuite(name, JUnitCounts(), JUnitTestCase[])
 
 mutable struct JUnitTestSuites
     const name::String
     counts::JUnitCounts
     testsuites::Vector{JUnitTestSuite}
 end
 
-JUnitTestSuites(name::String) = JUnitTestSuites(name, JUnitCounts(), JUnitTestSuite[])
+JUnitTestSuites(name::AbstractString) = JUnitTestSuites(name, JUnitCounts(), JUnitTestSuite[])
 
 function junit_record!(suites1::JUnitTestSuites, suites2::JUnitTestSuites)
     update!(suites1.counts, suites2.counts)

src/testcontext.jl

@@ -33,7 +33,7 @@ mutable struct TestContext
 end
 
 struct FileNode
-    path::String
+    path::SubString{String}
     testset::DefaultTestSet
     junit::Union{JUnitTestSuite,Nothing}
     testitems::Vector{TestItem} # sorted by line number within file
@@ -49,7 +49,7 @@ Base.push!(f::FileNode, ti::TestItem) = push!(f.testitems, ti)
 walk(f, fn::FileNode) = foreach(f, fn.testitems)
 
 struct DirNode
-    path::String
+    path::SubString{String}
     testset::DefaultTestSet
     junit::Union{JUnitTestSuites,Nothing}
     children::Vector{Union{FileNode, DirNode}} # sorted lexically by path

test/internals.jl

@@ -375,4 +375,27 @@ end
     end
 end
 
+@testset "nestedrelpath" begin
+    using ReTestItems: nestedrelpath
+    @assert Base.Filesystem.path_separator == "/"
+    path = "test/dir/foo_test.jl"
+    @test nestedrelpath(path, "test")  == relpath(path, "test")  == "dir/foo_test.jl"
+    @test nestedrelpath(path, "test/") == relpath(path, "test/") == "dir/foo_test.jl"
+    @test nestedrelpath(path, "test/dir")  == relpath(path, "test/dir")  == "foo_test.jl"
+    @test nestedrelpath(path, "test/dir/") == relpath(path, "test/dir/") == "foo_test.jl"
+    @test nestedrelpath(path, "test/dir/foo_test.jl") == relpath(path, "test/dir/foo_test.jl") == "."
+
+    # unlike `relpath`: if `startdir` is not a prefix of `path`, the assumption is violated,
+    # and `path` is just returned as-is
+    @test nestedrelpath(path, "test/dir/foo_") == "test/dir/foo_test.jl"
+    @test nestedrelpath(path, "test/dir/other") == "test/dir/foo_test.jl"
+    @test nestedrelpath(path, "test/dir/other/bar_test.jl") == "test/dir/foo_test.jl"
+
+    # leading '/' doesn't get ignored or stripped
+    @test nestedrelpath("/a/b/c", "/a/b") == "c"
+    @test nestedrelpath("/a/b/c", "a/b") == "/a/b/c"
+    @test nestedrelpath("/a/b", "/a/b/c") == "/a/b"
+    @test nestedrelpath("/a/b", "c") == "/a/b"
+end
+
 end # internals.jl testset