Stop adding $ as "terminators" when filtering tests for JUnit 5 #7080
Conversation
github-actions
bot
added
product: IntelliJ
|
@tpasternak could someone have a look at this as a follow up to #7060 and #7061? |
|
Sorry I'm out of the project for a few weeks already. Cc@sellophane |
| @@ -62,8 +63,9 @@ public void testProducedFromPsiMethod() throws Throwable { | |||
| (BlazeCommandRunConfiguration) fromContext.getConfiguration(); | |||
| assertThat(config.getTargets()) | |||
| .containsExactly(TargetExpression.fromStringSafe("//java/com/google/test:TestClass")); | |||
| String junit4Dollar = (jUnitVersionUnderTest == JUnitVersion.JUNIT_4 ? "$" : ""); | |||
There was a problem hiding this comment.
IMO such checks should be done in the base class.
There was a problem hiding this comment.
Do you mean something like adding a protected field in the base class so that it can be re-used?
There was a problem hiding this comment.
I don't like the fact that the accommodation for a particular version of JUnit (which in theory should be abstracted away by BlazeJUnitRunConfigurationProducerTestCase) leaks into the actual test cases. However, this isn't a big deal, and it shouldn't be an obstacle towards merging this PR.
There was a problem hiding this comment.
Me neither, but the reality is that the behaviour of the tool is different per JUnit version (and has questionable hardcoded coupled behaviour to Google's test runner), so we don't have much alternative to cover the different behaviours in the tests, in one way or another.
There was a problem hiding this comment.
However, this isn't a big deal, and it shouldn't be an obstacle towards merging this PR.
Do you want to approve it then?
While `$` may be useful in other JUnit versions to determine where a "class boundary" ends, as the plugin is using `,` for enumeration in that case (i.e. `mypackage.myClass1#myMethod1,myMethod2$|...`), it is not needed when the enumeration is between `()` as in JUnit5 (i.e. `mypackage.myClass1#(myMethod1|myMethod2)|...`).
github-actions
bot
removed
the
awaiting-review
Fixed by [7080](bazelbuild/intellij#7080) and released on [2025.03.04.0.1](https://github.com/bazelbuild/intellij/releases/tag/v2025.03.04-stable/).
Fixed by [7080](bazelbuild/intellij#7080) and released on [2025.03.04.0.1](https://github.com/bazelbuild/intellij/releases/tag/v2025.03.04-stable/).
Fixed by [7080](bazelbuild/intellij#7080) and released on [2025.03.04.0.1](https://github.com/bazelbuild/intellij/releases/tag/v2025.03.04-stable/).
While
$may be useful in other JUnit versions to determine where a "class boundary" ends, as the plugin is using,for enumeration in that case (i.e.mypackage.myClass1#myMethod1,myMethod2$|...), it is not needed when the enumeration is between()as in JUnit5 (i.e.mypackage.myClass1#(myMethod1|myMethod2)|...).Checklist
Please note that the maintainers will not be reviewing this change until all checkboxes are ticked. See
the Contributions section in the README for more
details.
Discussion thread for this change
Issue number: #7061
Description of this change