julia-vscode / TestItemRunner.jl

Run Julia test items
MIT License
67 stars 11 forks source link

Follow useful @testset behavior #51

Open aplavin opened 1 year ago

aplavin commented 1 year ago

Currently, @testitem behaves quite different compared to regular @testset. It would be better to follow the latter as possible, at least in some useful ways:

  1. Early stopping. One can do return in @testset, and the testset stops executing. Meanwhile, return seems to be a no-op in @testitem, quite surprisingly.
  2. Local scope. @testset run in local scope, while @testitem seems to have global scope. This difference manifests itself, for example, with warning for innocuous code like
    
    o = 1
    ...
    for i in 1:5
    o = i
    ...
    end

warns:

Warning: Assignment to o in soft scope is ambiguous because a global variable by the same name exists: o will be treated as a new local.



I think both can be solved by wrapping code generated by `@testitem` into `let ... end` blocks.
nickrobinson251 commented 1 year ago

In ReTestItems.jl, we think of @testitem more like a module than a testset (so 1 applies, but we've found it quite an intuitive change from testset). For 2, we "fix" that by running testitems with REPL-like "softscope".

NHDaly commented 11 months ago

To confirm: you aren't seeing the warnings you describe inside of a testitem, correct? We fix that via the change that Nick described.

For issue 1, yes, this is a notable departure from testsets, but it's an important one.

A testitem must be indepent, and completely isolated, so that it can be run in isolation from the VSCode interface, or in parallel, as part of runtests. For that reason, you must be able to define things like structs, functions, and modules, inside the testitem, which you cannot do in a testset.

^ This is the fundamental motivation for this difference. We try to highlight this difference in the docstrings. Please let us know if there is more that could be improved there. For example, maybe we can make this motivation more clear. Thanks!

aplavin commented 11 months ago

To confirm: you aren't seeing the warnings you describe inside of a testitem, correct? We fix that via the change that Nick described.

I see exactly this warning with testitems, but not testset - that's why I report it here.

Create runtests.jl with this content:

using TestItems
using TestItemRunner
@run_package_tests

@testitem "x" begin
    o = 1
    for i in 1:5
        o = i
    end
end

and run it either through ]test or through vs code test runner, and get the warning:

┌ Warning: Assignment to `o` in soft scope is ambiguous because a global variable by the same name exists: `o` will be treated as a new local. Disambiguate by using `local o` to suppress this warning or `global o` to assign to the existing global variable.
aplavin commented 11 months ago

For issue 1, yes, this is a notable departure from testsets, but it's an important one.

A testitem must be indepent, and completely isolated, so that it can be run in isolation from the VSCode interface, or in parallel, as part of runtests. For that reason, you must be able to define things like structs, functions, and modules, inside the testitem, which you cannot do in a testset.

Yes, I understand this part, but how is it related to my point 1? I'm talking about early stopping that is a useful feature to have in a testset/testitem. In testset, one can do return and the test set stops executing. But neither this, nor anything else I tried, doesn't stop @testitem execution.