Add option to throw rather than warn if given a useless path (#131)

* Throw rather than warn if given a useless path

* Make path validation opt-in

* Move path validation to own function

* Add tests

* Bump version

Author: nickrobinson251

Project.toml

@@ -1,6 +1,6 @@
 name = "ReTestItems"
 uuid = "817f1d60-ba6b-4fd5-9520-3cf149f6a823"
-version = "1.21.0"
+version = "1.22.0"
 
 [deps]
 Dates = "ade2ca70-3891-5945-98fb-dc099432e06a"

src/ReTestItems.jl

@@ -99,6 +99,22 @@ function _validated_nworker_threads(str)
     return replace(str, "auto" => string(Sys.CPU_THREADS))
 end
 
+function _validated_paths(paths, should_throw::Bool)
+    return filter(paths) do p
+        if !ispath(p)
+            msg = "No such path $(repr(p))"
+            should_throw ? throw(ArgumentError(msg)) : @warn msg
+            return false
+        elseif !(is_test_file(p) || is_testsetup_file(p)) && isfile(p)
+            msg = "$(repr(p)) is not a test file"
+            should_throw ? throw(ArgumentError(msg)) : @warn msg
+            return false
+        else
+            return true
+        end
+    end
+end
+
 """
     ReTestItems.runtests()
     ReTestItems.runtests(mod::Module)
@@ -168,8 +184,12 @@ will be run.
   For interative sessions, `:eager` is the default when running with 0 or 1 worker processes, `:batched` otherwise.
   For non-interactive sessions, `:issues` is used by default.
 - `verbose_results::Bool`: If `true`, the final test report will list each `@testitem`, otherwise
-    the results are aggregated. Default is `false` for non-interactive sessions
-    or when `logs=:issues`, `true` otherwise.
+  the results are aggregated. Default is `false` for non-interactive sessions
+  or when `logs=:issues`, `true` otherwise.
+- `validate_paths::Bool=false`: If `true`, `runtests` will throw an error if any of the
+  `paths` passed to it cannot contain test files, either because the path doesn't exist or
+  the path points to a file which is not a test file. Default is `false`.
+  Can also be set using the `RETESTITEMS_VALIDATE_PATHS` environment variable.
 """
 function runtests end
 
@@ -214,19 +234,11 @@ function runtests(
     logs::Symbol=Symbol(get(ENV, "RETESTITEMS_LOGS", default_log_display_mode(report, nworkers))),
     verbose_results::Bool=(logs !== :issues && isinteractive()),
     test_end_expr::Expr=Expr(:block),
+    validate_paths::Bool=parse(Bool, get(ENV, "RETESTITEMS_VALIDATE_PATHS", "false")),
 )
     nworker_threads = _validated_nworker_threads(nworker_threads)
-    paths′ = filter(paths) do p
-        if !ispath(p)
-            @warn "No such path $(repr(p))"
-            return false
-        elseif !(is_test_file(p) || is_testsetup_file(p)) && isfile(p)
-            @warn "$(repr(p)) is not a test file"
-            return false
-        else
-            return true
-        end
-    end
+    paths′ = _validated_paths(paths, validate_paths)
+
     logs in LOG_DISPLAY_MODES || throw(ArgumentError("`logs` must be one of $LOG_DISPLAY_MODES, got $(repr(logs))"))
     report && logs == :eager && throw(ArgumentError("`report=true` is not compatible with `logs=:eager`"))
     (0 ≤ memory_threshold ≤ 1) || throw(ArgumentError("`memory_threshold` must be between 0 and 1, got $(repr(memory_threshold))"))

test/integrationtests.jl

@@ -79,26 +79,39 @@ end
     @test n_passed(results) == 1
 end
 
-@testset "Warn when not test file" begin
+@testset "Warn or error when not test file" begin
     pkg = joinpath(TEST_PKG_DIR, "TestsInSrc.jl")
 
     # warn if the path does not exist
     dne = joinpath(pkg, "does_not_exist")
-    @test_logs (:warn, "No such path \"$dne\"") match_mode=:any begin
+    dne_msg = "No such path \"$dne\""
+    @test_logs (:warn, dne_msg) match_mode=:any begin
         runtests(dne)
     end
+    # throw if `validate_paths`
+    @test_throws ArgumentError(dne_msg) runtests(dne; validate_paths=true)
+    # test setting `validate_paths` via environment variable
+    withenv("RETESTITEMS_VALIDATE_PATHS" => 1) do
+        @test_throws ArgumentError(dne_msg) runtests(dne)
+    end
 
     # warn if the file is not a test file
     file = joinpath(pkg, "src", "foo.jl")
     @assert isfile(file)
-    @test_logs (:warn, "\"$file\" is not a test file") match_mode=:any begin
+    file_msg = "\"$file\" is not a test file"
+    @test_logs (:warn, file_msg) match_mode=:any begin
         runtests(file)
     end
+    # throw if `validate_paths`
+    @test_throws ArgumentError(file_msg) runtests(file; validate_paths=true)
 
     # Warn for each invalid path
-    @test_logs (:warn, "No such path \"$dne\"") (:warn, "\"$file\" is not a test file") match_mode=:any begin
+    @test_logs (:warn, dne_msg) (:warn, file_msg) match_mode=:any begin
         runtests(dne, file)
     end
+    # Throw on first invalid path if `validate_paths`
+    @test_throws ArgumentError(dne_msg) runtests(dne, file; validate_paths=true)
+    @test_throws ArgumentError(file_msg) runtests(file, dne; validate_paths=true)
 
     # Warn for each invalid path and still run valid ones
     test_file = joinpath(pkg, "src", "foo_test.jl")
@@ -109,6 +122,8 @@ end
         end
     end
     @test n_tests(results) == 2 # foo_test.jl has 2 tests
+    # Throw on first invalid path, even if some are valid, if `validate_paths`
+    @test_throws ArgumentError(dne_msg) runtests(test_file, dne, file; validate_paths=true)
 end
 
 @testset "filter `runtests(func, x)`" begin

test/internals.jl

@@ -249,4 +249,36 @@ end
     end
 end
 
+@testset "_validated_paths" begin
+    _validated_paths = ReTestItems._validated_paths
+    testfiles_dir = joinpath(pkgdir(ReTestItems), "test", "testfiles")
+
+    test_file = joinpath(testfiles_dir, "_happy_tests.jl")
+    @assert isfile(test_file)
+    for _throw in (false, true)
+        @test _validated_paths((test_file,), _throw) == (test_file,)
+        @test_logs _validated_paths((test_file,), _throw) # test nothing is logged
+    end
+
+    @assert !ispath("foo")
+    @test _validated_paths(("foo",), false) == ()
+    @test_logs (:warn, "No such path \"foo\"") _validated_paths(("foo",), false)
+    @test_throws ArgumentError("No such path \"foo\"") _validated_paths(("foo",), true)
+
+    @assert isfile(test_file)
+    @assert !ispath("foo")
+    paths = (test_file, "foo",)
+    @test _validated_paths(paths, false) == (test_file,)
+    @test_logs (:warn, "No such path \"foo\"") _validated_paths(paths, false)
+    @test_throws ArgumentError("No such path \"foo\"") _validated_paths(paths, true)
+
+    nontest_file = joinpath(testfiles_dir, "_empty_file.jl")
+    @assert isfile(nontest_file)
+    @assert !ReTestItems.is_test_file(nontest_file)
+    @assert !ReTestItems.is_testsetup_file(nontest_file)
+    @test _validated_paths((nontest_file,), false) == ()
+    @test_logs (:warn, "\"$nontest_file\" is not a test file") _validated_paths((nontest_file,), false)
+    @test_throws ArgumentError("\"$nontest_file\" is not a test file") _validated_paths((nontest_file,), true)
+end
+
 end # internals.jl testset