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

Take into account TaskDefs with TestSelectors #108

Merged
merged 2 commits into from
Dec 29, 2021

Conversation

kpodsiad
Copy link
Contributor

As far as I understand, TestSelector can be used to create TaskDef which should only execute selected tests from the given test class.

Motivation: from the build server perspective I want to be able to run only a single test from the selected test suite.

I didn't find a better alternative to RunSettings.testFilter.

@kpodsiad
Copy link
Contributor Author

@eed3si9n what do you think about this approach?

@eed3si9n
Copy link
Member

Motivation: from the build server perspective I want to be able to run only a single test from the selected test suite.

Could you elaborate what you mean here please? afaik junit-interface distinguishes what's passed into testOnly with or without --, and is currently capable of filtering either to a test class or test method.

By build server, do you mean an implementation of BSP server?
In general is there a spec or an informal consensus around this?
Also, would this change affect backward compatibility?

@kpodsiad
Copy link
Contributor Author

Yeah, sure. I'll start from the very beginning, just to ensure you have the necessary context. It was my mistake not doing it from the very beginning.

I wanted to implement vscode's testing API for Scala in Metals and I managed to implement running whole test suites/classes via BSP's debug method and DAP. Since all well-known build servers such as sbt or bloop (there's also mill) use the test-interface to offload testing, it was quite an easy job. All I had to do was extract necessary information from sbt.testing.Event.

Having finer granulation of tests is my next goal. However, it turned out that it's not that easy since most of the already implemented test interfaces don't take into TestSelector into account (I'm speaking about junit, munit and utest among others), only scalatest does it. By not taking TestSelector into account I mean ignoring the content of the TaskDef's _selectors field.
Even so, I have to admit that this fact is reasonable since those interfaces were created for sbt purposes and they fits exactly what sbt needs from them.

Now I should be able to answer your first question which was:

Motivation: from the build server perspective I want to be able to run only a single test from the selected test suite.

Could you elaborate what you mean here please? afaik junit-interface distinguishes what's passed into testOnly with or without --, and is currently capable of filtering either to a test class or test method.

So here, I'm not talking from the sbt testOnly perspective, I'm talking from the build server (bloop or sbt) perspective (although it could be any tool which offloads testing via the test-interface) which can only use TaskDef and Task to specify what tests are going to be run. (TaskDef's which are passed to the ForkMain)
Ok, to be honest, I guess I can also specify separate arguments per framework but approach with TestSelector seems more universal. Every framework has different way of specyfing test names, like for exmaple it is --tests=$REGEX for junit.

By build server, do you mean an implementation of BSP server?

Yes

In general is there a spec or an informal consensus around this?

There is a proposition to extend BSP debug method with a new request kind which will provide necessary information about tests that are about to run.
I did a quick PoC in Bloop where I used information passed in this new request to alter TaskDefs which are sent to the ForkMain and changes in this PR are the result of my experiment.

I started with the JUnit since it's the easiest one to start because test methods are annotated - hence methods names are easy to find and pass into this whole machinery.

Also, would this change affect backward compatibility?

No, it shouldn't since this change doesn't affect the way in which sbt or bloop uses junit-interface currently (they specify only SuiteSelector in TaskDef)

@eed3si9n
Copy link
Member

Thanks for the context. I guess having the concept encoded as test-interface class sounds as good as a spec.

Do you think it's possible to implement some test, or it's not possible as a scripted test (if it's too cumbersome it's fine)?

@kpodsiad
Copy link
Contributor Author

kpodsiad commented Dec 28, 2021

It may be hard to test this via a scripted test.

If JUnitTask has a method that returns the value of RunSettings.testFilter then it'll be possible to check if this setting is altered correctly. Adding a method only for test purposes doesn't seem like an elegant solution, but it's the only idea I have.

Actually, I have an idea how to test this.

@kpodsiad
Copy link
Contributor Author

Scripted test implemented, it tests both previous behavior and the new one.

Copy link
Member

@eed3si9n eed3si9n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@eed3si9n eed3si9n merged commit f8c6372 into sbt:develop Dec 29, 2021
@eed3si9n
Copy link
Member

https://github.com/sbt/junit-interface/releases/tag/v0.13.3 is on its way to Maven Central.

@kpodsiad
Copy link
Contributor Author

Thanks, @eed3si9n!

kpodsiad pushed a commit to kpodsiad/munit that referenced this pull request Mar 12, 2022
kpodsiad pushed a commit to kpodsiad/munit that referenced this pull request Mar 12, 2022
kpodsiad pushed a commit to kpodsiad/munit that referenced this pull request Mar 12, 2022
kpodsiad pushed a commit to kpodsiad/munit that referenced this pull request Mar 12, 2022
kpodsiad pushed a commit to kpodsiad/munit that referenced this pull request Mar 12, 2022
kpodsiad pushed a commit to kpodsiad/munit that referenced this pull request Mar 20, 2022
kpodsiad added a commit to scalameta/munit that referenced this pull request Mar 21, 2022
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