Skip to content

Conversation

@theogf
Copy link
Owner

@theogf theogf commented Jun 30, 2025

This would replace #56

This adds the interface for TestItems. I needed a couple of changes in the interface, more precisely the arguments passed to expr_transform.

@disberd do you think you could give it a try?

@disberd
Copy link

disberd commented Jul 1, 2025

Hi @theogf, nice to see the update!

I had a quick try and found two issues:

  • running a file directly does not work. This is because the current file execution part only does include the file in a testset but there is no preamble detection (so no using TestItemRunner) for testitems.
  • I can't seem to get fzf detection of testitems when trying to filter on the testitem label directly (I simply get an empty list). This I didn't found out why from a very preliminary look at the code

As an additional side note, I believe that for TestItemRunner specifically, loading the preamble within the same begin end of the test content might be problematic if the file containing the testitem also relies on @testsnippet or @testmodule.
Macro expansion would in fact happen before the using statement is executed resulting in an error as below. I think in general for packages that relies on macro they define, you would need to execute the using in the preamble at top level before evaling the other preambles found within the file
image

@theogf
Copy link
Owner Author

theogf commented Jul 6, 2025

running a file directly does not work. This is because the current file execution part only does include the file in a testset but there is no preamble detection (so no using TestItemRunner) for testitems.

I would expect that, this is the same problem as for #54

I can't seem to get fzf detection of testitems when trying to filter on the testitem label directly (I simply get an empty list). This I didn't found out why from a very preliminary look at the code

That's a bigger problem, could you remind me which package you were trying this on? I could experiment there directly.

As an additional side note, I believe that for TestItemRunner specifically, loading the preamble within the same begin end of the test content might be problematic if the file containing the testitem also relies on @testsnippet or @testmodule.

Is it possible that one should load TestItems and not TestItemsRunner to get the macros?

@disberd
Copy link

disberd commented Jul 10, 2025

Hi @theogf, sorry for the late reply.

That's a bigger problem, could you remind me which package you were trying this on? I could experiment there directly.

I had tried it on the first package I had at hand but didn't do much investigation on this. Let me try back more consistently when I have a bit more time and come back at you with some more concrete example if the problem indeed persists.

Is it possible that one should load TestItems and not TestItemsRunner to get the macros?

TestItemRunners also exports those macro definition. The problem is more fundamental and as I tried to show with the screenshot example above, stems from the fact that the current preamble injection model adds the preamble expression does so as first expression within a begin ... end block:

preamble_statements = prepend_preamble_statements(interface, Expr.(preamble))
ex = Expr(:block, preamble_statements..., tried_testset)
EvalTest(ex, test_info)

You can't have a macro being imported and then used within the same block (as also discussed in https://github.com/JuliaLang/julia/issues/46478)

And that is also the reason why in #56 I added the import statements at the top level of the eval module rather than within the EvalBlock:
https://github.com/disberd/TestPicker.jl/blob/6646bdba6ee2ba8e52bbb529512468b2c608afce/src/eval.jl#L54-L56

It would be good if this interface would support a way to specify whether a preamble statement should be put at top level instead of within the begin...end block. You could probably also take the easier way of always putting the interface specific preamble statements at the top level of the generated module as in any case a separate module is generated for each ran test as I understand it

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

Successfully merging this pull request may close these issues.

3 participants