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

Defining testsets inside a function not supported? #44

Open
fingolfin opened this issue Jan 3, 2022 · 5 comments
Open

Defining testsets inside a function not supported? #44

fingolfin opened this issue Jan 3, 2022 · 5 comments

Comments

@fingolfin
Copy link
Member

For OSCAR, we use "testset templates" to provide generic conformance for various "interfaces" based on Test.
These currently come in the form of a function that takes an object to be tested, and which then internally produces a @testset carrying out the test. The idea is that other packages then can use these conformance tests on their own implementations of those interfaces/protocols.

Here is a severely stripped down example:

function test_Ring_interface(R::AbstractAlgebra.Ring)

   T = elem_type(R)

   @testset "Ring interface for $(R) of type $(typeof(R))" begin

      @test T <: RingElement

      # ... more tests come here

   end
end

Unfortunately, with ReTest, this doesn't seem to be supported? Trying to use ReTest with the above resulted in this:

...
    nested task error: TaskFailedException

        nested task error: UndefVarError: R not defined
        Stacktrace:
          [1] macro expansion
            @ ~/.julia/packages/ReTest/WnRIG/src/testset.jl:505 [inlined]
          [2] macro expansion
            @ ~/Projekte/OSCAR/AbstractAlgebra.jl/test/Rings-conformance-tests.jl:205 [inlined]
          [3] top-level scope
            @ ~/.julia/packages/ReTest/WnRIG/src/ReTest.jl:517
          [4] eval
            @ ./boot.jl:360 [inlined]
...

I.e. it isn't capturing the surrounding context.

Is there any chance this could be fixed? If so, how complicated do you think it would be / where would one have to start?

I am also open to alternative suggestions for implementing such conformance tests; but the current approach is quite nice in that it is easy to grok what it does.

@Sacha0
Copy link

Sacha0 commented Jan 21, 2022

Chances are you've seen it, but just in case not, this section of the documentation discusses this issue and provides a mechanism that might do the trick :).

@rfourquet
Copy link
Collaborator

Yeah it's an open problem for ReTest. In a branch I've tried improving the API around @testset_macro that Sacha mentioned, but I had a hard time working around the fact that repeated invocations of @macroexpand1 are not equivalent to @macroexpand, but I don't have the details in my head right now.

In any case, a solution would probably involve annotating the function containing @testsets with a macro. If this is not possible (e.g. in the case where you want to keep the tests compatible with the Test stdlib), I'm not sure whether a solution exists. Having the function always use Test.@testset instead ot ReTest.@testset could be a compromise, but at the moment they are not very well integrated together (even though this should already work to a certain extent when using ReTest.hijack).

@Sacha0
Copy link

Sacha0 commented Feb 26, 2022

I imagine you've thought of something like iterating calls to runtests till a fixed point is reached Rafael, and discarded the idea? :)

@rfourquet
Copy link
Collaborator

That's a good idea to explore more, but basically the problem is that there might be legitimate use cases where you want to have a function define toplevel testsets. In the OP example, that's not the case, and you want the newly defined testset to be a child of the currently running testset. This pattern works with Test because running testsets are stored in a task-local storage, and that's the mecanism used to define "what is a child testset".
In ReTest, a child testset is by definition a testset which is syntactically enclosed within another testset (the parent). In other words, the core of the difference relevant to this issue is that Test.@testset is similar to "dynamically scoped" variables (like in some lisps), vs ReTest.testset which works like syntactically scoped variable.

And BTW, the issue is very simliar to using include in testsets from the other issue. That's why I would rather prefer an explicit marker stating the intent "make it work as with Test", i.e. make this a child testset of the currently running testset".

For include, a specific testset_include (or perhaps @testset_include?) function would do and is probably relatively easy to implement.

For functions, the current workaround is to use macros. The slightly better API, mentioned in my previous comment above, that I tried to implement was something like @testset "subject" macro(x) ... end instead of having to make a regular macro and then register it with @testset_macro. But definitely, a macro is not as convenient to write as a function. So maybe your idea could be made to work, i.e. detecting newly defined testset during a retest run and running them immediatly. Ideally in a way that the example in the OP, can also work, i.e. capturing local variables and interpolate them in testset subjects. But this would indeed have to work as Test.@testset, i.e. such children testset would need to not be recorded permanently as children, because on the next run of the parent, the function will again be called and define fresh children. As this is already partly implemented with hijack, maybe just supporting functions defining Test.@testsets would be good enough (or at least a decent stopgap). Possibly wrapped as ReTest.@testset dyncamic=true ....

In any case, I need to dive again into this problem :) Last time I gave up after one non-conclusive day of work, to focus on more urgent features.

@Sacha0
Copy link

Sacha0 commented Feb 27, 2022

Cheers, thanks for the thoughtful response Rafael! :)

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