Skip to content

Improve desc_filter so you can select deeply nested tests #2077

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

Merged
merged 6 commits into from
Aug 8, 2025

Conversation

kubajal
Copy link
Contributor

@kubajal kubajal commented Apr 7, 2025

Currently, desc_filter let's you:

  • run either a single test_that or a single describe expressions and not an it expression
  • run jest the top-level test_that and describe expressions

This PR expands desc_filter capabilities by adding support for:

This is implemented by making desc_filter accept list of test labels that act as selectors in the test hierarchy.

So e.g. running testthat::test_file("tests.R", desc=c("level 0, "level 1 A", "level 2 B"))

# tests.R
f()
describe("level 0", {
  g()
  describe("level 1", {
    h()
    it("level 2 A", {}),
    i()
    it("level 2 B", {})
    j()
})

effectively triggers just

f()
describe("level 0", {
  g()
  describe("level 1", {
    h()
    i()
    it("level 2 B", {})
})

This gives more room for good IDE integrations, e.g. fixing meakbiyik/vscode-r-test-adapter#24.

This PR is a continuation of #1904 and #1840.

@kubajal kubajal force-pushed the feature/desc_filter_improvement branch from 4af569a to 84dc618 Compare April 8, 2025 19:57
@kubajal
Copy link
Contributor Author

kubajal commented Apr 9, 2025

The error on Ubuntu is not related to my change and has been observed in other PRs too, e.g. #2061:

Error: Error: running the tests in ‘testthat.R’ failed
Error: Error: Failure in `/home/runner/work/_temp/package/testthat/testthat-tests/testthat.Rout.fail`
r ('test-local.R:38:3'): can override translation of error messages ─────
<notSubsettableError/error/condition>
Error in `mean[[1]]`: object of type 'closure' is not subsettable
Backtrace:

@kubajal
Copy link
Contributor Author

kubajal commented May 9, 2025

@hadley could you have a look? This PR is ready for review.

@hadley
Copy link
Member

hadley commented May 12, 2025

@kubajal I'll take a look when I'm next working on testthat.

@hadley
Copy link
Member

hadley commented Jul 23, 2025

I'm not sure that this makes sense to me, given that there's often code run outside of it() blocks in describe(), so I don't think there's much guarantee that this will work in general, unfortunately.

@hadley hadley closed this Jul 23, 2025
@kubajal
Copy link
Contributor Author

kubajal commented Jul 25, 2025

Perhaps my description was misleading. With my changes, any code outside of it(...) statement does get executed. It's just that the only it(...) statement that does get executed is the one with the provided desc argument.

Here are my 3 arguments to merge it:

  • this PR will enable better IDE support for running just the failing assertions instead of running the whole suite of tests, e.g. in https://github.com/meakbiyik/vscode-r-test-adapter which I'm a contributor of and I would be interested in improving UX of testthat in VSCode
  • the new code logic in this PR carefully mimics mechanism of selection of single describe() / testthat() statements but in a more granular way
  • the changes still trigger the code run outside of it() statements

@kubajal
Copy link
Contributor Author

kubajal commented Jul 25, 2025

@hadley could you please revisit this PR and the comment above?

@hadley
Copy link
Member

hadley commented Jul 28, 2025

In that case, I'd suggest:

  • You make desc take a character vector rather than introducing an obscure new separator.
  • You work on minimising the difference with the existing code so its easier for me to review.
  • You include a test demonstrating the your code preserves other non-it() code blocks.

@hadley hadley reopened this Jul 28, 2025
@kubajal
Copy link
Contributor Author

kubajal commented Jul 28, 2025

Thanks, will fix that.

@kubajal kubajal force-pushed the feature/desc_filter_improvement branch 6 times, most recently from 26cdea5 to f2ea828 Compare August 1, 2025 16:14
@kubajal kubajal force-pushed the feature/desc_filter_improvement branch from f2ea828 to c04eb3d Compare August 1, 2025 16:40
@kubajal kubajal changed the title Improve desc_filter so you can run single BDD-style tests (aka run just a single it test from describe) Improve desc_filter so you can select deeply nested tests Aug 1, 2025
@kubajal
Copy link
Contributor Author

kubajal commented Aug 1, 2025

@hadley the PR is ready for a new review.

I also updated the title and description of the PR so it is more precise.

@hadley
Copy link
Member

hadley commented Aug 6, 2025

Ok, I've reimplemented it in a way that's more easy for me to understand and refined all the tests.

@jennybc can you please take a look? This shouldn't change any existing behaviour but should enable recursive selection of tests inside other tests.

@hadley hadley requested a review from jennybc August 6, 2025 14:58
@hadley
Copy link
Member

hadley commented Aug 7, 2025

Fixes #2118

@hadley hadley merged commit 596b3e4 into r-lib:main Aug 8, 2025
13 checks passed
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