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.

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

retest requires you are located in the test/ dir of the package being tested (at least if using ReTest.hijack) #29

Open
nickrobinson251 opened this issue Aug 3, 2021 · 6 comments

Comments

@nickrobinson251
Copy link

...i think that's what's going on :)

Here's the error i ran into when trying out ReTest.jl for the first time (using ReTest v0.3.0)

julia> using ChainRules, ReTest

julia> ReTest.hijack(ChainRules)
Testing ChainRules.jl
Main.ChainRulesTests

julia> retest()
                          Pass
Main.ChainRulesTests:
Testing test_helpers.jl:
                          Pass   Error   Total
  ChainRules          |              1       1

ChainRules: Error During Test at /Users/nick/.julia/packages/ReTest/63MpE/src/ReTest.jl:440
  Got exception outside of a @test
  SystemError: opening file "/Users/nick/repos/ChainRules.jl/test_helpers.jl": No such file or directory
  Stacktrace:
    [1] systemerror(p::String, errno::Int32; extrainfo::Nothing)
      @ Base ./error.jl:174
    [2] #systemerror#68
      @ ./error.jl:173 [inlined]
    [3] systemerror
      @ ./error.jl:173 [inlined]
    [4] open(fname::String; lock::Bool, read::Nothing, write::Nothing, create::Nothing, truncate::Nothing, append::Nothing)
      @ Base ./iostream.jl:293
    [5] open
      @ ./iostream.jl:282 [inlined]
    [6] open(f::Base.var"#354#355"{String}, args::String; kwargs::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
      @ Base ./io.jl:328
    [7] open
      @ ./io.jl:328 [inlined]
    [8] read
      @ ./io.jl:436 [inlined]
    [9] _include(mapexpr::Function, mod::Module, _path::String)
      @ Base ./loading.jl:1249
   [10] include(mod::Module, _path::String)
      @ Base ./Base.jl:417
   [11] include
      @ ~/.julia/packages/ReTest/63MpE/src/hijack.jl:194 [inlined]
   [12] macro expansion
      @ ./timing.jl:210 [inlined]
   [13] include_test(path::String)
      @ Main.ChainRulesTests ~/repos/ChainRules.jl/test/runtests.jl:20
   [14] macro expansion
      @ ~/repos/ChainRules.jl/test/runtests.jl:25 [inlined]
   [15] macro expansion
      @ ~/.julia/packages/ReTest/63MpE/src/testset.jl:647 [inlined]
   [16] macro expansion
      @ ~/repos/ChainRules.jl/test/runtests.jl:25 [inlined]
   [17] top-level scope
      @ ~/.julia/packages/ReTest/63MpE/src/ReTest.jl:440
   [18] eval
      @ ./boot.jl:373 [inlined]
   [19] #73
      @ ~/.julia/packages/ReTest/63MpE/src/ReTest.jl:1083 [inlined]
   [20] #137
      @ /Users/julia/buildbot/worker/package_macos64/build/usr/share/julia/stdlib/v1.7/Distributed/src/remotecall.jl:354 [inlined]
   [21] run_work_thunk(thunk::Distributed.var"#137#138"{ReTest.var"#73#91"{ReTest.Testset.Format}, Tuple{Module, ReTest.TestsetExpr, ReTest.And, NamedTuple{(:out, :compute, :preview), Tuple{Distributed.RemoteChannel{Channel{Union{Nothing, ReTest.Testset.ReTestSet}}}, Channel{Nothing}, Nothing}}}, Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}}}, print_error::Bool)
      @ Distributed /Users/julia/buildbot/worker/package_macos64/build/usr/share/julia/stdlib/v1.7/Distributed/src/process_messages.jl:63
   [22] remotecall_fetch(::Function, ::Distributed.LocalProcess, ::Module, ::Vararg{Any}; kwargs::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
      @ Distributed /Users/julia/buildbot/worker/package_macos64/build/usr/share/julia/stdlib/v1.7/Distributed/src/remotecall.jl:379
   [23] remotecall_fetch(::Function, ::Distributed.LocalProcess, ::Module, ::Vararg{Any})
      @ Distributed /Users/julia/buildbot/worker/package_macos64/build/usr/share/julia/stdlib/v1.7/Distributed/src/remotecall.jl:379
   [24] remotecall_fetch(::Function, ::Int64, ::Module, ::Vararg{Any}; kwargs::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
      @ Distributed /Users/julia/buildbot/worker/package_macos64/build/usr/share/julia/stdlib/v1.7/Distributed/src/remotecall.jl:421
   [25] remotecall_fetch
      @ /Users/julia/buildbot/worker/package_macos64/build/usr/share/julia/stdlib/v1.7/Distributed/src/remotecall.jl:421 [inlined]
   [26] macro expansion
      @ ~/.julia/packages/ReTest/63MpE/src/ReTest.jl:1080 [inlined]
   [27] (::ReTest.var"#71#89"{Int64, ReTest.Testset.Format, Nothing, ReTest.Testset.ReTestSet, Base.Threads.Atomic{Bool}, Channel{Nothing}, Distributed.RemoteChannel{Channel{Union{Nothing, ReTest.Testset.ReTestSet}}}, Vector{Any}, ReTest.And, Module})()
      @ ReTest ./task.jl:411

ERROR: Some tests did not pass: 0 passed, 0 failed, 1 errored, 0 broken.
@rfourquet
Copy link
Collaborator

Thanks for the report!
I'm afraid there is a long way before having ReTest.hijack support ChainRules tests.

The issue at hand here is basically that a @testset containing include is hard for ReTest. There is partial (unsatisfying) support via tha very badly named keyword arg testset=true in hijack. It could be extended to accept other names than include, e.g. have testset=:include_test in the case of ChainRules. Without that, the helper test file is not found, because the include context of "test/runtests.jl", which is normally active when loading "runtests.jl", is lost with ReTest: the top-level @testset expressions are evaluated after the whole content of "runtests.jl" has been evaluated into a ChainRulesTests module. I'm not sure if there is a fix for this (like being able to manually set the include context when calling retest()).

The next problem is the use in ChainRulesTestUtils of functions declaring @testsets within them. As mentioned in the Caveats section in ReTest's docs, this is not supported for now. There is possibly a work-around if you are willing to switch fully to ReTest via @testset_macro.

Actually both problems are related: ReTest basically requires "lexical" nesting of @testsets, in order to statically inspect them, whereas the body of a Test.@testset can call functions (like include or test_ functions in ChainRulesTestUtils) which declare @testsets which will dynamically be interpreted as nested testsets. So this is an open problem for ReTest. For hijack, maybe the simplest course of action would be to make dynamically added testsets behave as Test.@testset. This won't allow ReTest to know about them ahead of time, but at least they would have a chance to run without complications.

Then, another complication here is that ChainRulesTestUtils also depends on Test, and I don't believe we could make hijack understand @testset's defined in there. The solution is likely to do like mentioned above, make ReTest.@testset and Test.@testset play nice together, i.e. allow the latter being (dynamically) nested within the former. This would break the alignment in the report, and not allow retest to know statically about Test.@testsets for filtering, but I think this could be made to work without too much difficulty.

rfourquet added a commit that referenced this issue Aug 6, 2021
rfourquet added a commit that referenced this issue Aug 6, 2021
rfourquet added a commit that referenced this issue Aug 6, 2021
rfourquet added a commit that referenced this issue Aug 6, 2021
@rfourquet
Copy link
Collaborator

rfourquet commented Aug 9, 2021

I think this should be fixed on the master branch, if you like to try it out, you can use ReTest.hijack(ChainRules, include_functions=[:include_test], include=:static) (it could help with finding out bugs). I will tag a release with this functionality in a few days.

@nickrobinson251
Copy link
Author

Thanks for workign on this!

Using current master (via ] add ReTest#master), the suggested command errors for me:

(@v1.6) pkg> activate ~/repos/ChainRules.jl
  Activating environment at `~/repos/ChainRules.jl/Project.toml`

julia> using ChainRules, ReTest
[ Info: Precompiling ChainRules [082447d4-558c-5d27-93f4-14fc19e9eca2]

julia> ReTest.hijack(ChainRules, include_functions=[:include_test], include=:static)
ERROR: LoadError: ArgumentError: Package ChainRulesTestUtils not found in current path:
- Run `import Pkg; Pkg.add("ChainRulesTestUtils")` to install the ChainRulesTestUtils package.

Stacktrace:
 [1] require(into::Module, mod::Symbol)
   @ Base ./loading.jl:893
 [2] include(mapexpr::Function, mod::Module, _path::String)
   @ Base ./Base.jl:387
 [3] include(mapexpr::Function, x::String)
   @ Main.ChainRulesTests ~/.julia/packages/ReTest/Oh6iK/src/hijack.jl:224
 [4] top-level scope
   @ ~/.julia/packages/ReTest/Oh6iK/src/hijack.jl:260
 [5] eval
   @ ./boot.jl:360 [inlined]
 [6] populate_mod!(mod::Module, path::String; lazy::Bool, Revise::Nothing, include::Symbol, include_functions::Vector{Symbol})
   @ ReTest ~/.julia/packages/ReTest/Oh6iK/src/hijack.jl:257
 [7] hijack(path::String, modname::Symbol; parentmodule::Module, lazy::Bool, revise::Nothing, include::Symbol, testset::Bool, include_functions::Vector{Symbol})
   @ ReTest ~/.julia/packages/ReTest/Oh6iK/src/hijack.jl:225
 [8] hijack(packagemod::Module, modname::Nothing; parentmodule::Module, lazy::Bool, revise::Nothing, include::Symbol, testset::Bool, include_functions::Vector{Symbol})
   @ ReTest ~/.julia/packages/ReTest/Oh6iK/src/hijack.jl:302
 [9] top-level scope
   @ REPL[3]:1
in expression starting at /Users/nick/repos/ChainRules.jl/test/runtests.jl:4
ReTest v0.3.0 `https://github.com/JuliaTesting/ReTest.jl.git#master`

@nickrobinson251
Copy link
Author

Ah, but if i'm in an env with access to the test-only dependencies (e.g. via TestEnv.jl), then this works!

(@v1.6) pkg> activate ~/repos/ChainRules.jl
  Activating environment at `~/repos/ChainRules.jl/Project.toml`
  
julia> using ChainRules, ReTest, TestEnv

julia> TestEnv.activate()
"/var/folders/hx/1h0bbkfd18d4n1qrnwmrl4j00000gn/T/jl_AoHxyk/Project.toml"

julia> using ChainRules, ReTest, TestEnv

julia> ReTest.hijack(ChainRules, include_functions=[:include_test], include=:static)
[ Info: Precompiling ChainRulesTestUtils [cdddcdb0-9152-4a09-a978-84456f9df70a]
Testing ChainRules.jl
Main.ChainRulesTests

julia> retest()
Testing /Users/nick/repos/ChainRules.jl/test/test_helpers.jl:
  0.013368 seconds (3.02 k allocations: 199.380 KiB, 39.03% compilation time)
Testing /Users/nick/repos/ChainRules.jl/test/rulesets/Core/core.jl:
...
...

@rfourquet
Copy link
Collaborator

Yes I also used TestEnv for testing this, thanks @oxinabox !
I also noticed that retest has a 2.5s delay when working with ChainRules test code, which must be the biggest I played with. I hope this will be bearable, but I believe there will be a fix for this eventually.

@oxinabox
Copy link
Member

oxinabox commented Aug 9, 2021

ReTest.jl needing to be in the test environment was the thing that brought me over the edge to creating TestEnv.jl.
I had wanted it for a while, but this was the straw that broke the camel's back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants