Skip to content

Conversation

@disberd
Copy link

@disberd disberd commented Jun 25, 2025

This PR is an attempt to fully support @testitem objects from the VSCode testing framework.

The feature added in #47 unfortunately is only useful for detecting @testitem objects in the fuzzy picker, but will throw an error when trying to actually execute those tests.

I wanted to have the nice capability of this package for interactive filtering of @testitem objects (to which I migrated most of my packages recently).

The current implementation is very simple but relies on the addition of TestItemRunner as a dependency. I thought about having this as an extension but the code modification is quite simpler when accepting this as direct dependency and wanted first to get a feedback from @theogf on whether such an additional feature would even be considered acceptable before putting more effort into a potential extension :D.

The code now is very simply mostly because it relies on actually calling the relevant testitem detection and execution from TestItemRunner.

I am just defining a new @testitem macro that internally calls TestItemRunner.run_tests so that the detected @testitem expressions can be left as they are parsed

I had asked on slack whether the TestItemRunner.run_tests can be considered public on this discussion which I am also attaching as screenshot to avoid the slackhole:
image

As mentioned on slack by @davidanthoff, the good long term solution would probably be to do something more official directly in the julia-vscode organization but that is definitely something more complex and I think this might still be a useful short-term solution (even if not merged, people can at least specifically install this PR's branch to try it out)

Edit: And here is a short video of the new functionality in action:

a8b79bf8-707d-470b-be23-6bfc648aaad8.mp4

@codecov
Copy link

codecov bot commented Jun 25, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 47.31%. Comparing base (0ec996c) to head (6646bdb).

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #56      +/-   ##
==========================================
+ Coverage   46.15%   47.31%   +1.16%     
==========================================
  Files           7        7              
  Lines         312      317       +5     
==========================================
+ Hits          144      150       +6     
+ Misses        168      167       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@theogf
Copy link
Owner

theogf commented Jun 25, 2025

Hey! First of all thanks for your contribution it's always great to have external PRs!

Regarding the changes, I am not very keen in adding an additional dependency to TestPicker.jl, so I would not approve this PR as is.
However! This could work perfectly as a package extension! But this means that some rework needs to be done with regard to the interface, especially with the variety of needs of other packages like TestItemRunner.jl.

I actually have a need for such a flexibly interface myself for work so I would be very keen to design an API that would allow most package to:

  • Filter for some specific blocks.
  • Define a custom preamble (as you do here).
  • Perform an AST transform before evaluation.

This would also help formalize some aspects of the code and help with e.g. #49

@disberd
Copy link
Author

disberd commented Jun 25, 2025

Hey @theogf, thanks for the quick reaction!

I imagined this would not be merged as is (and I understand), I just needed something and was curious to use TestPicker with @testitem as I really like the idea of the package (thanks for developing it!).
Since it was so easy to implement I though to open a PR to make it more visible to people who might be interested in having this functionality till it's more robustly available either directly in TestItemRunner or as an extension after your planned chnages

The planned upgrade in API you hinted at seems a very nice improvement and indeed I'll just park this until you implement your upgrade.
If you prefer to just close this directly instead of leaving it open that is fine for me.

Additionally, if you want to get an additional pair of eyes on any draft PR for your planned upgrade feel free to ping me!

@theogf
Copy link
Owner

theogf commented Jun 25, 2025

The planned upgrade in API you hinted at seems a very nice improvement and indeed I'll just park this until you implement your upgrade.
If you prefer to just close this directly instead of leaving it open that is fine for me.

Additionally, if you want to get an additional pair of eyes on any draft PR for your planned upgrade feel free to ping me!

I think I am done writing the interface now. I will open a draft PR but I still need to write a few more tests to test it on something new and then I will make a PR. Your review will be very appreciated!

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.

2 participants