Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP JET integration idea #134

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

WIP JET integration idea #134

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
50c561b
Very WIP JET integration idea
Drvi Jan 8, 2024
ec14054
Make JET a weak dependency
Drvi Jan 9, 2024
0f9d85e
[WIP] Introduce `test/packages.PkgUsingJET.jl`
Drvi Jan 9, 2024
db97988
Progress
Drvi Jan 12, 2024
06c906f
Add a way for a hard opt-out per test item
Drvi Jan 12, 2024
2b974e2
Fix JET report filtering
Drvi Jan 15, 2024
3b52add
More fixes and tweaks
Drvi Jan 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ Sockets = "6462fe0b-24de-5631-8697-dd941f90decc"
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
TestEnv = "1e6cf692-eddd-4d53-88a5-2d735e33781b"

[weakdeps]
JET = "c3a54625-cd67-489e-a8e7-0a5a0ff4e31b"

[extensions]
JETExt = "JET"

[compat]
Dates = "1"
Logging = "1"
Expand Down
36 changes: 36 additions & 0 deletions ext/JETExt.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
module JETExt

import JET, ReTestItems, Test

function _analyze_toplevel(ex, file, mode)
toplevelex = Expr(:toplevel, ex)
analyzer = JET.JETAnalyzer(; mode)
config = JET.ToplevelConfig(; mode)
res = JET.virtual_process(toplevelex, file, analyzer, config)
return JET.JETToplevelResult(analyzer, res, "analyze_toplevel"; mode)

Check warning on line 10 in ext/JETExt.jl

View check run for this annotation

Codecov / codecov/patch

ext/JETExt.jl#L5-L10 

Added lines #L5 - L10 were not covered by tests
end

function ReTestItems.jet_test(ti::ReTestItems.TestItem, mod_expr::Expr)
ti.jet == :none && return nothing

Check warning on line 14 in ext/JETExt.jl

View check run for this annotation

Codecov / codecov/patch

ext/JETExt.jl#L13-L14 

Added lines #L13 - L14 were not covered by tests

onfail(::Function, ::Test.Pass) = nothing
onfail(f::Function, ::Test.Fail) = f()

Check warning on line 17 in ext/JETExt.jl

View check run for this annotation

Codecov / codecov/patch

ext/JETExt.jl#L16-L17 

Added lines #L16 - L17 were not covered by tests

@assert mod_expr.head === :module "Expected the test item expression to be wrapped in a module, got $(repr(mod_expr.head))"
Test.@testset "JET $(repr(ti.jet)) mode" begin
result = _analyze_toplevel(mod_expr, ti.file, ti.jet)
no_jet_errors = isempty(result)
onfail(Test.@test no_jet_errors) do
JET.print_reports(

Check warning on line 24 in ext/JETExt.jl

View check run for this annotation

Codecov / codecov/patch

ext/JETExt.jl#L19-L24 

Added lines #L19 - L24 were not covered by tests
stdout,
JET.get_reports(result),
# Remove the name of the module JET uses for virtualization the code and the name of the module
# we wrap the test items in.
JET.gen_postprocess(result.res.actual2virtual) ∘ x->replace(x, string("var\"", mod_expr.args[2], "\"") => ""),

Check warning on line 29 in ext/JETExt.jl

View check run for this annotation

Codecov / codecov/patch

ext/JETExt.jl#L29 

Added line #L29 was not covered by tests
)
end
end
return nothing

Check warning on line 33 in ext/JETExt.jl

View check run for this annotation

Codecov / codecov/patch

ext/JETExt.jl#L33 

Added line #L33 was not covered by tests
end

end
13 changes: 11 additions & 2 deletions src/ReTestItems.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@
# The provided `worker_num` is only for logging purposes, and not persisted as part of the worker.
function start_worker(proj_name, nworker_threads, worker_init_expr, ntestitems; worker_num=nothing)
w = Worker(; threads="$nworker_threads")
i = worker_num == nothing ? "" : " $worker_num"
i = worker_num === nothing ? "" : " $worker_num"
# remote_fetch here because we want to make sure the worker is all setup before starting to eval testitems
remote_fetch(w, quote
using ReTestItems, Test
Expand Down Expand Up @@ -980,7 +980,7 @@
# environment = Module()
@debugv 1 "Running test item $(repr(name))$(_on_worker())."
_, stats = @timed_with_compilation _redirect_logs(logs == :eager ? DEFAULT_STDOUT[] : logpath(ti)) do
with_source_path(() -> Core.eval(Main, mod_expr), ti.file)
with_source_path(() -> (Core.eval(Main, mod_expr); jet_test(ti, mod_expr)), ti.file)
with_source_path(() -> Core.eval(Main, test_end_mod_expr), ti.file)
nothing # return nothing as the first return value of @timed_with_compilation
end
Expand Down Expand Up @@ -1037,4 +1037,13 @@

convert_results_to_be_transferrable(x) = x

# This method signature must be less specific than the overload in ext/JETExt.jl
function jet_test(ti::Any, mod_expr::Expr)
ti.jet == :none && return nothing
Test.@testset "JET package extension failure: JET not loaded, ignoring \"jet=$(repr(ti.jet))\"" begin
Test.@test_broken false

Check warning on line 1044 in src/ReTestItems.jl

View check run for this annotation

Codecov / codecov/patch

src/ReTestItems.jl#L1043-L1044 

Added lines #L1043 - L1044 were not covered by tests
end
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@nickrobinson251 What do you think about recording a broken test instead of an error? We'll hit this method whenever JET is not loaded in the session evaluating the test items, so e.g. if the user wants a faster run locally without JET analysis, they'd get just a bunch of broken tests instead of noisy errors.

Copy link
Collaborator

Choose a reason for hiding this comment

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

if running via runtests, that'll activate the test environment which will then presumably load JET (if that test suite has Jet tests), right? How would you run a test suite that has JET as a test dependency without causing JET to get loaded?

i wonder if we add a runtests keyword skip_jet::Bool=false, which when true causes all JET tests to be skipped (i.e. any testitem with jet=... will record a skipped test, regardless of whether or not JET is loaded)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Activating the environment doesn't load its packages, one needs to import JET or using JET on the worker before we try to use the extension, so even if the suite has JET tests, if those are not eval'd first, we won't have the extension available for all test items. This is why I wonder -- do we want the user to manually preload JET via init_worker_expr or do we want to preload the package for them?

The runtest-level keyword is a good idea 👍 and I think that apart skip_jet we might also want default_jet::Symbol=:none (with corresponding ENV variables), so that the user doesn't have to edit a bunch of test items to run JET on all of them.

Copy link
Collaborator

Choose a reason for hiding this comment

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

hmmm, interesting. what if, when a testitem sets the jet keyword, the runtestitem code adds using JET to the test code (just like how it automatically adds using Test and using $PackageBeingTested)? Would that work.

we might also want default_jet::Symbol=:none (with corresponding ENV variables),

Agreed. I'd be inclined to just name this jet::Symbol... maybe even we allow jet=:skip as the way to skip JET tests?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

what if, when a testitem sets the jet keyword, the runtestitem code adds using JET to the test code (just like how it automatically adds using Test and using $PackageBeingTested)? Would that work.

Precisely! Ok, thanks, I'll look into it 👍

jet::Symbol... maybe even we allow jet=:skip

💯

return nothing

Check warning on line 1046 in src/ReTestItems.jl

View check run for this annotation

Codecov / codecov/patch

src/ReTestItems.jl#L1046 

Added line #L1046 was not covered by tests
end

end # module ReTestItems
11 changes: 9 additions & 2 deletions src/macros.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,17 +125,18 @@
line::Int
project_root::String
code::Any
jet::Symbol
testsetups::Vector{TestSetup} # populated by runtests coordinator
workerid::Base.RefValue{Int} # populated when the test item is scheduled
testsets::Vector{DefaultTestSet} # populated when the test item is finished running
eval_number::Base.RefValue{Int} # to keep track of how many items have been run so far
stats::Vector{PerfStats} # populated when the test item is finished running
scheduled_for_evaluation::ScheduledForEvaluation # to keep track of whether the test item has been scheduled for evaluation
end
function TestItem(number, name, id, tags, default_imports, setups, retries, timeout, skip, file, line, project_root, code)
function TestItem(number, name, id, tags, default_imports, setups, retries, timeout, skip, file, line, project_root, code, jet)
_id = @something(id, repr(hash(name, hash(relpath(file, project_root)))))
return TestItem(
number, name, _id, tags, default_imports, setups, retries, timeout, skip, file, line, project_root, code,
number, name, _id, tags, default_imports, setups, retries, timeout, skip, file, line, project_root, code, jet,
TestSetup[],
Ref{Int}(0),
DefaultTestSet[],
Expand Down Expand Up @@ -250,6 +251,7 @@
setup = Any[]
skip = false
_id = nothing
jet = QuoteNode(:none)
_run = true # useful for testing `@testitem` itself
_source = QuoteNode(__source__)
if length(exs) > 1
Expand Down Expand Up @@ -293,6 +295,10 @@
elseif kw == :_source
_source = ex.args[2]
@assert isa(_source, Union{QuoteNode,Expr})
elseif kw == :jet
jet = ex.args[2]
@assert isa(jet, QuoteNode)
@assert jet in QuoteNode.((:none, :typo, :basic, :sound))

Check warning on line 301 in src/macros.jl

View check run for this annotation

Codecov / codecov/patch

src/macros.jl#L298-L301 

Added lines #L298 - L301 were not covered by tests
else
error("unknown `@testitem` keyword arg `$(ex.args[1])`")
end
Expand All @@ -309,6 +315,7 @@
$String($_source.file), $_source.line,
$gettls(:__RE_TEST_PROJECT__, "."),
$q,
$jet,
)
if !$_run
$ti
Expand Down
2 changes: 1 addition & 1 deletion test/internals.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ end # `include_testfiles!` testset
@testset "report_empty_testsets" begin
using ReTestItems: TestItem, report_empty_testsets, PerfStats, ScheduledForEvaluation
using Test: DefaultTestSet, Fail, Error
ti = TestItem(Ref(42), "Dummy TestItem", "DummyID", [], false, [], 0, nothing, false, "source/path", 42, ".", nothing)
ti = TestItem(Ref(42), "Dummy TestItem", "DummyID", [], false, [], 0, nothing, false, "source/path", 42, ".", nothing, :none)

ts = DefaultTestSet("Empty testset")
report_empty_testsets(ti, ts)
Expand Down
2 changes: 1 addition & 1 deletion test/log_capture.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ end
@testset "log capture -- reporting" begin
setup1 = @testsetup module TheTestSetup1 end
setup2 = @testsetup module TheTestSetup2 end
ti = TestItem(Ref(42), "TheTestItem", "ID007", [], false, [], 0, nothing, false, "source/path", 42, ".", nothing)
ti = TestItem(Ref(42), "TheTestItem", "ID007", [], false, [], 0, nothing, false, "source/path", 42, ".", nothing, :none)
push!(ti.testsetups, setup1)
push!(ti.testsetups, setup2)
push!(ti.testsets, Test.DefaultTestSet("dummy"))
Expand Down
226 changes: 226 additions & 0 deletions test/packages/PkgUsingJET.jl/Manifest.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,226 @@
# This file is machine-generated - editing it directly is not advised

julia_version = "1.10.0"
manifest_format = "2.0"
project_hash = "acba4ed3ade3cccc8954146b28d1516bf39ebd32"

[[deps.ArgTools]]
uuid = "0dad84c5-d112-42e6-8d28-ef12dabb789f"
version = "1.1.1"

[[deps.Artifacts]]
uuid = "56f22d72-fd6d-98f1-02f0-08ddc0907c33"

[[deps.Base64]]
uuid = "2a0f44e3-6c83-55bd-87e4-b1978d98bd5f"

[[deps.CodeTracking]]
deps = ["InteractiveUtils", "UUIDs"]
git-tree-sha1 = "c0216e792f518b39b22212127d4a84dc31e4e386"
uuid = "da1fd8a2-8d9e-5ec2-8556-3022fb5608a2"
version = "1.3.5"

[[deps.Dates]]
deps = ["Printf"]
uuid = "ade2ca70-3891-5945-98fb-dc099432e06a"

[[deps.Distributed]]
deps = ["Random", "Serialization", "Sockets"]
uuid = "8ba89e20-285c-5b6f-9357-94700520ee1b"

[[deps.Downloads]]
deps = ["ArgTools", "FileWatching", "LibCURL", "NetworkOptions"]
uuid = "f43a241f-c20a-4ad4-852c-f6b1247861c6"
version = "1.6.0"

[[deps.FileWatching]]
uuid = "7b1f6079-737a-58dc-b8bc-7a2ca5c1b5ee"

[[deps.InteractiveUtils]]
deps = ["Markdown"]
uuid = "b77e0a4c-d291-57a0-90e8-8db25a27a240"

[[deps.JET]]
deps = ["InteractiveUtils", "JuliaInterpreter", "LoweredCodeUtils", "MacroTools", "Pkg", "PrecompileTools", "Preferences", "Revise", "Test"]
git-tree-sha1 = "9587e44f478b5fddc70fc3baae60a587deaa3a31"
uuid = "c3a54625-cd67-489e-a8e7-0a5a0ff4e31b"
version = "0.8.24"

[[deps.JuliaInterpreter]]
deps = ["CodeTracking", "InteractiveUtils", "Random", "UUIDs"]
git-tree-sha1 = "04663b9e1eb0d0eabf76a6d0752e0dac83d53b36"
uuid = "aa1ae85d-cabe-5617-a682-6adf51b2e16a"
version = "0.9.28"

[[deps.LibCURL]]
deps = ["LibCURL_jll", "MozillaCACerts_jll"]
uuid = "b27032c2-a3e7-50c8-80cd-2d36dbcbfd21"
version = "0.6.4"

[[deps.LibCURL_jll]]
deps = ["Artifacts", "LibSSH2_jll", "Libdl", "MbedTLS_jll", "Zlib_jll", "nghttp2_jll"]
uuid = "deac9b47-8bc7-5906-a0fe-35ac56dc84c0"
version = "8.4.0+0"

[[deps.LibGit2]]
deps = ["Base64", "LibGit2_jll", "NetworkOptions", "Printf", "SHA"]
uuid = "76f85450-5226-5b5a-8eaa-529ad045b433"

[[deps.LibGit2_jll]]
deps = ["Artifacts", "LibSSH2_jll", "Libdl", "MbedTLS_jll"]
uuid = "e37daf67-58a4-590a-8e99-b0245dd2ffc5"
version = "1.6.4+0"

[[deps.LibSSH2_jll]]
deps = ["Artifacts", "Libdl", "MbedTLS_jll"]
uuid = "29816b5a-b9ab-546f-933c-edad1886dfa8"
version = "1.11.0+1"

[[deps.Libdl]]
uuid = "8f399da3-3557-5675-b5ff-fb832c97cbdb"

[[deps.Logging]]
uuid = "56ddb016-857b-54e1-b83d-db4d58db5568"

[[deps.LoggingExtras]]
deps = ["Dates", "Logging"]
git-tree-sha1 = "c1dd6d7978c12545b4179fb6153b9250c96b0075"
uuid = "e6f89c97-d47a-5376-807f-9c37f3926c36"
version = "1.0.3"

[[deps.LoweredCodeUtils]]
deps = ["JuliaInterpreter"]
git-tree-sha1 = "0b8cf121228f7dae022700c1c11ac1f04122f384"
uuid = "6f1432cf-f94c-5a45-995e-cdbf5db27b0b"
version = "2.3.2"

[[deps.MacroTools]]
deps = ["Markdown", "Random"]
git-tree-sha1 = "b211c553c199c111d998ecdaf7623d1b89b69f93"
uuid = "1914dd2f-81c6-5fcd-8719-6d5c9610ff09"
version = "0.5.12"

[[deps.Markdown]]
deps = ["Base64"]
uuid = "d6f4376e-aef5-505a-96c1-9c027394607a"

[[deps.MbedTLS_jll]]
deps = ["Artifacts", "Libdl"]
uuid = "c8ffd9c3-330d-5841-b78e-0817d7145fa1"
version = "2.28.2+1"

[[deps.MozillaCACerts_jll]]
uuid = "14a3606d-f60d-562e-9121-12d972cd8159"
version = "2023.1.10"

[[deps.NetworkOptions]]
uuid = "ca575930-c2e3-43a9-ace4-1e988b2c1908"
version = "1.2.0"

[[deps.OrderedCollections]]
git-tree-sha1 = "dfdf5519f235516220579f949664f1bf44e741c5"
uuid = "bac558e1-5e72-5ebc-8fee-abe8a469f55d"
version = "1.6.3"

[[deps.Pkg]]
deps = ["Artifacts", "Dates", "Downloads", "FileWatching", "LibGit2", "Libdl", "Logging", "Markdown", "Printf", "REPL", "Random", "SHA", "Serialization", "TOML", "Tar", "UUIDs", "p7zip_jll"]
uuid = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f"
version = "1.10.0"

[[deps.PrecompileTools]]
deps = ["Preferences"]
git-tree-sha1 = "03b4c25b43cb84cee5c90aa9b5ea0a78fd848d2f"
uuid = "aea7be01-6a6a-4083-8856-8a6e6704d82a"
version = "1.2.0"

[[deps.Preferences]]
deps = ["TOML"]
git-tree-sha1 = "00805cd429dcb4870060ff49ef443486c262e38e"
uuid = "21216c6a-2e73-6563-6e65-726566657250"
version = "1.4.1"

[[deps.Printf]]
deps = ["Unicode"]
uuid = "de0858da-6303-5e67-8744-51eddeeeb8d7"

[[deps.REPL]]
deps = ["InteractiveUtils", "Markdown", "Sockets", "Unicode"]
uuid = "3fa0cd96-eef1-5676-8a61-b3b8758bbffb"

[[deps.Random]]
deps = ["SHA"]
uuid = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"

[[deps.ReTestItems]]
deps = ["Dates", "Logging", "LoggingExtras", "Pkg", "Serialization", "Sockets", "Test", "TestEnv"]
path = "../../.."
uuid = "817f1d60-ba6b-4fd5-9520-3cf149f6a823"
version = "1.23.0"
weakdeps = ["JET"]

[deps.ReTestItems.extensions]
JETExt = "JET"

[[deps.Requires]]
deps = ["UUIDs"]
git-tree-sha1 = "838a3a4188e2ded87a4f9f184b4b0d78a1e91cb7"
uuid = "ae029012-a4dd-5104-9daa-d747884805df"
version = "1.3.0"

[[deps.Revise]]
deps = ["CodeTracking", "Distributed", "FileWatching", "JuliaInterpreter", "LibGit2", "LoweredCodeUtils", "OrderedCollections", "Pkg", "REPL", "Requires", "UUIDs", "Unicode"]
git-tree-sha1 = "3fe4e5b9cdbb9bbc851c57b149e516acc07f8f72"
uuid = "295af30f-e4ad-537b-8983-00126c2a3abe"
version = "3.5.13"

[[deps.SHA]]
uuid = "ea8e919c-243c-51af-8825-aaa63cd721ce"
version = "0.7.0"

[[deps.Serialization]]
uuid = "9e88b42a-f829-5b0c-bbe9-9e923198166b"

[[deps.Sockets]]
uuid = "6462fe0b-24de-5631-8697-dd941f90decc"

[[deps.TOML]]
deps = ["Dates"]
uuid = "fa267f1f-6049-4f14-aa54-33bafae1ed76"
version = "1.0.3"

[[deps.Tar]]
deps = ["ArgTools", "SHA"]
uuid = "a4e569a6-e804-4fa4-b0f3-eef7a1d5b13e"
version = "1.10.0"

[[deps.Test]]
deps = ["InteractiveUtils", "Logging", "Random", "Serialization"]
uuid = "8dfed614-e22c-5e08-85e1-65c5234f0b40"

[[deps.TestEnv]]
deps = ["Pkg"]
git-tree-sha1 = "c35f69c951ac4f74b8b074f62dfb1e169b351497"
uuid = "1e6cf692-eddd-4d53-88a5-2d735e33781b"
version = "1.101.1"

[[deps.UUIDs]]
deps = ["Random", "SHA"]
uuid = "cf7118a7-6976-5b1a-9a39-7adc72f591a4"

[[deps.Unicode]]
uuid = "4ec0a83e-493e-50e2-b9ac-8f72acf5a8f5"

[[deps.Zlib_jll]]
deps = ["Libdl"]
uuid = "83775a58-1f1d-513f-b197-d71354ab007a"
version = "1.2.13+1"

[[deps.nghttp2_jll]]
deps = ["Artifacts", "Libdl"]
uuid = "8e850ede-7688-5339-a07c-302acd2aaf8d"
version = "1.52.0+1"

[[deps.p7zip_jll]]
deps = ["Artifacts", "Libdl"]
uuid = "3f19e933-33d8-53b3-aaab-bd5110c3b7a0"
version = "17.4.0+2"
16 changes: 16 additions & 0 deletions test/packages/PkgUsingJET.jl/Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
name = "PkgUsingJET"
uuid = "c3f2fd6f-de40-45a4-991e-5ffd14a5ac2f"
authors = ["Tomáš Drvoštěp <tomas.drvostep@gmail.com>"]
version = "0.1.0"

[deps]
JET = "c3a54625-cd67-489e-a8e7-0a5a0ff4e31b"
ReTestItems = "817f1d60-ba6b-4fd5-9520-3cf149f6a823"

[extras]
JET = "c3a54625-cd67-489e-a8e7-0a5a0ff4e31b"
ReTestItems = "817f1d60-ba6b-4fd5-9520-3cf149f6a823"
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"

[targets]
test = ["JET", "ReTestItems", "Test"]
5 changes: 5 additions & 0 deletions test/packages/PkgUsingJET.jl/src/PkgUsingJET.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module PkgUsingJET

greet() = print("Hello World!")

end # module PkgUsingJET
Loading
Loading