Skip to content

Commit

Permalink
Error if test files don't only have @testitem/@testsetup (#78)
Browse files Browse the repository at this point in the history
* Error if test files don't only have `@testitem`/`@testsetup`

* Fix line number in test

* Check for `@testset`/`@test` specifically

* Add tests

* fixup! Check for `@testset`/`@test` specifically

* Bump version
  • Loading branch information
nickrobinson251 authored Jun 7, 2023
1 parent f5c6c91 commit c346b25
Show file tree
Hide file tree
Showing 11 changed files with 60 additions and 14 deletions.
2 changes: 1 addition & 1 deletion Project.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name = "ReTestItems"
uuid = "817f1d60-ba6b-4fd5-9520-3cf149f6a823"
version = "1.6.0"
version = "1.7.0"

[deps]
Dates = "ade2ca70-3891-5945-98fb-dc099432e06a"
Expand Down
38 changes: 37 additions & 1 deletion src/ReTestItems.jl
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,42 @@ function _is_subproject(dir, current_projectfile)
return true
end

# Error if we are trying to `include` a file with anything except an `@testitem` or
# `@testsetup` call at the top-level.
# Re-use `Base.include` to avoid duplicating subtle code-loading logic from `Base`.
# Note:
# For now this is just checks for `:macrocall` so we can support alternative macros that
# expand to be an `@testitem`. We will likely _tighten_ this check in future.
# i.e. We may in future throw an error for files that currently successfully get included.
# i.e. Only `@testitem` and `@testsetup` calls are officially supported.
checked_include(mod, filepath) = Base.include(check_retestitem_macrocall, mod, filepath)
function check_retestitem_macrocall(expr)
is_retestitem_macrocall(expr) || _throw_not_macrocall(expr)
return expr
end
function is_retestitem_macrocall(expr::Expr)
if expr.head == :macrocall
# For now, we're not checking for `@testitem`/`@testsetup` only,
# but we can still guard against the most common issue.
name = expr.args[1]
if name != Symbol("@testset") && name != Symbol("@test")
return true
end
end
return false
end
function _throw_not_macrocall(expr)
# `Base.include` sets the `:SOURCE_PATH` before the `mapexpr`
# (`check_retestitem_macrocall`) is first called
file = get(task_local_storage(), :SOURCE_PATH, "unknown")
msg = """
Test files must only include `@testitem` and `@testsetup` calls.
In $(repr(file)) got:
$(Base.remove_linenums!(expr))
"""
error(msg)
end

# for each directory, kick off a recursive test-finding task
function include_testfiles!(project_name, projectfile, paths, shouldrun, report::Bool)
project_root = dirname(projectfile)
Expand Down Expand Up @@ -557,7 +593,7 @@ function include_testfiles!(project_name, projectfile, paths, shouldrun, report:
task_local_storage(:__RE_TEST_ITEMS__, $file_node) do
task_local_storage(:__RE_TEST_PROJECT__, $(project_root)) do
task_local_storage(:__RE_TEST_SETUPS__, $setups) do
Base.include(Main, $filepath)
checked_include(Main, $filepath)
end
end
end
Expand Down
10 changes: 9 additions & 1 deletion test/integrationtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ end

@testset "Warn on empty test set -- integration test" begin
@test_logs (:warn, """
Test item "Warn on empty test set -- integration test" at test/testfiles/_empty_testsets_tests.jl:4 contains test sets without tests:
Test item "Warn on empty test set -- integration test" at test/testfiles/_empty_testsets_tests.jl:1 contains test sets without tests:
"Empty testset"
"Inner empty testset"
""") match_mode=:any begin
Expand Down Expand Up @@ -692,4 +692,12 @@ end
@test !occursin("Info message from testitem", captured.output)
end

@testset "error on code outside `@testitem`/`@testsetup`" begin
err_msg = "Test files must only include `@testitem` and `@testsetup` calls."
expected = VERSION < v"1.8" ? Exception : err_msg
@test_throws expected runtests(joinpath(TEST_FILES_DIR, "_misuse_file1_test.jl"))
@test_throws expected runtests(joinpath(TEST_FILES_DIR, "_misuse_file2_test.jl"))
@test_throws expected runtests(joinpath(TEST_FILES_DIR, "_misuse_file3_test.jl"))
end

end # integrationtests.jl testset
2 changes: 0 additions & 2 deletions test/testfiles/_abort_tests.jl
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
using ReTestItems

@testitem "Abort" begin
# we're explicitly crashing the worker here
ccall(:abort, Cvoid, ())
Expand Down
3 changes: 0 additions & 3 deletions test/testfiles/_empty_testsets_tests.jl
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
using Test: @testset
using ReTestItems: @testitem

@testitem "Warn on empty test set -- integration test" begin
@testset "Empty testset" begin end

Expand Down
2 changes: 0 additions & 2 deletions test/testfiles/_filter_tests.jl
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
using ReTestItems

@testitem "Test item no tags" begin
@test true
end
Expand Down
2 changes: 0 additions & 2 deletions test/testfiles/_logging_test.jl
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
using ReTestItems

@testitem "Test that uses a logger" begin
@info "Info message from testitem"
@test true
Expand Down
3 changes: 3 additions & 0 deletions test/testfiles/_misuse_file1_test.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
@testset "not a testitem" begin
@test true
end
5 changes: 5 additions & 0 deletions test/testfiles/_misuse_file2_test.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
@testitem "pass" begin
@test true
end

@test 1 + 1 == 2
5 changes: 5 additions & 0 deletions test/testfiles/_misuse_file3_test.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
using ReTestItems

@testitem "foo" begin
@test true
end
2 changes: 0 additions & 2 deletions test/testfiles/_timeout_tests.jl
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
using ReTestItems

@testitem "Test item takes 60 seconds" begin
sleep(60.0)
@test true
Expand Down

2 comments on commit c346b25

@nickrobinson251
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JuliaRegistrator
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Registration pull request created: JuliaRegistries/General/85086

After the above pull request is merged, it is recommended that a tag is created on this repository for the registered package version.

This will be done automatically if the Julia TagBot GitHub Action is installed, or can be done manually through the github interface, or via:

git tag -a v1.7.0 -m "<description of version>" c346b2558042e5db966a20d3652baec1f34886b9
git push origin v1.7.0

Please sign in to comment.