intellij
intellij copied to clipboard
bazelbuild
Allow running single Scalatest tests in Intellij.
ScalaTestContextProvider is responsible for figuring out which parameters to pass to the Scalatest runner. It contains code capable of deciding which test class to run, but not which individual test case in the class to run.
This commit replaces the Bazel plugin's own logic for this with a call to the corresponding logic in the Scala plugin, which is capable of deciding both the class and test name.
When no test name is detected (e.g. due to running a whole class), the plugin will continue passing the class name in --test_filter as before.
When a test name is detected, the plugin will instead set --test_arg=-t --test_arg="testName", and leave --test_filter out. This is because rules_scala turns --test_filter into the -s Scalatest runner parameter, which selects a full class for execution. Passing both -s and -t would cause Scalatest to run both the full class, as well as the selected test.
Fixes #3425
@tpasternak Yes, I attempted this but gave up. The test infrastructure in that test fakes too much. The current test in that class defines a couple of interfaces that simulate the ones in Scalatest closely enough to work for the class lookup code.
The find-the-test code in the Scala plugin relies on an "org.scalatest.Finders" annotation on the test interface, such as the one on AnyFunSuite @Finders(Array("org.scalatest.finders.FunSuiteFinder")), which then points to a class in Scalatest that contains logic on how to find the relevant class.
I think in order to add a test for this, I'd have to define a workspace that actually depends on Scalatest, I can't just fake a project with some dummy interfaces. I didn't spot any good tooling to do this, or any tests that did something similar, so I decided to drop it.
What do you think?
My actual problem is that in order to figure out how to make a test work here, I need to debug into code from the Scala IJ plugin, specifically ScalaTestConfigurationProducer, but when I open this project in IJ, sources are not attached for the scalaCommunity jar.
If you can tell me how to fix that, I can probably get some kind of test working.
I'm not sure, it would be good to have integration tests for this code, but at the moment it would be indeed hard to do it due to limitations of the test framework you have found. Let me ask someone about it.
On the other hand, maybe it would be good not to split code into "class run" and "single test run". In this case, we would always use addBlazeFlagsModification. This would mean, we would cover whole configuration builder logic by in the test class test case.
In the meantime, I noticed there's a bug here. The Run Configuration executes all tests of the specified name, even if they are in separate suites. For example:
import org.scalatest.flatspec.AnyFlatSpec
import org.scalatest.matchers.must.Matchers
class ExampleTest extends AnyFlatSpec with Matchers {
"Test1" should "pass" in {
2 must be(2)
}
"Test2" should "pass" in {
2 mustBe 2
}
}
class ExampleTest22 extends AnyFlatSpec with Matchers {
"Test1" should "pass" in {
}
}
In the meantime, I noticed there's a bug here. The Run Configuration executes all tests of the specified name, even if they are in separate suites. For example:
Yes, I noticed this and initially thought it was an issue with ScalaTest, so I just left it in. I tried passing both --test_filter and -t, and the result was that both the specified test, and all tests in the specified class were run.
It turns out it's actually an oversight in rules_scala. rules_scala turns --test_filter=className into an -s className flag for ScalaTest's runner, putting it at the end of whichever other arguments are specified.
ScalaTest cares about the ordering of these flags. "-s className -t testName" means "run the test in className with the specified name". "-t testName -s className" means "run all tests in className and also run tests that match the specified name".
I'll submit an issue to rules_scala and see how they feel about moving -s to the front of the parameter list instead of the back. I don't think this behavior is on purpose. If we can make the change there, this plugin can switch to passing both --test_filter and -t at some point. For now I've made it use --test_filter only when running a class, while running a single test specifies -s and -t via --test_arg flags instead.
On the other hand, maybe it would be good not to split code into "class run" and "single test run". In this case, we would always use addBlazeFlagsModification. This would mean, we would cover whole configuration builder logic by in the test class test case.
I gave making a test for this another shot, and it turns out that the Scala plugin's code for figuring out which single test is being pointed at depends on loading the test class via a classloader, and then reading the annotation I mentioned above. I'm pretty sure the test in BlazeScalaTestClassConfigurationProducerTest doesn't even compile the test class, so we'd need to figure out another way to test it.
Regarding not splitting, I can merge the two paths together now if you're happy with not setting --test_filter and passing the relevant -s flag via --test_arg instead?
@tpasternak I've updated the code to always use addBlazeFlagsModification. I don't really see the point of setting --test_filter over setting the -s flag directly, so I've merged the two code paths so the flag modification always sets -s, and only sets -t if a test case is selected.
This should work on any rules_scala version.
Ok, LGTM
@mai93 @jastice we don't have integration tests for this, but it seems we can't develop them due to limitations of current tests framework. Can we merge this?
@mai93 @jastice we don't have integration tests for this, but it seems we can't develop them due to limitations of current tests framework. Can we merge this?
Thanks! I think that's fine.
Thank you @srdo-humio I've a question about this case:
import org.scalatest.WordSpec
import scala.collection.mutable.Stack
class StackSpec extends WordSpec {
"A Stack" should {
"pop values in last-in-first-out order" in {
val stack = new Stack[Int]
stack.push(1)
stack.push(2)
assert(stack.pop() === 2)
assert(stack.pop() === 1)
}
"throw NoSuchElementException if an empty stack is popped" in {
val emptyStack = new Stack[String]
intercept[NoSuchElementException] {
emptyStack.pop()
}
}
}
}
When I try to run the test from "A Stack" should { line, the methods are not shown in the Test Results window in the bottom left of the screen. Have you checked if this can be fixed?
When I try to run the test from
"A Stack" should {line, the methods are not shown in the Test Results window in the bottom left of the screen. Have you checked if this can be fixed?
No, I haven't. Is this an issue introduced with these changes, or was it also broken before?
I'm asking because I don't know where IJ gets the listing for the test results window from, and it wasn't my intent to change anything about that window. If it's a regression I'll figure out how to fix it. If it was broken before these changes as well, I think it should be addressed separately.
No, I haven't. Is this an issue introduced with these changes, or was it also broken before?
It was not broken before because it used to run the whole test class independent of from where you hit the run tests button. So it used to always list all the tests it ran.
It was not broken before because it used to run the whole test class independent of from where you hit the run tests button. So it used to always list all the tests it ran.
Okay, I looked at it a bit, and it appears that when running tests from that line, the test name ends up being the concatenation of both test names, so it fails to run.
From peeking at the scala plugin source, I see that scalaTestConfigurationProducer.getTestClassWithTestName can actually return multiple tests, it squishes the test names together with newlines and returns the whole string as the "testName" field in the ClassAndTestName. I think we just need to separate the names again and pass a -t per test to fix this. I'll look into it once I'm back in the office.
That being said, I think it was also broken before, just in a different way. Before you could only run the whole class. Now you can run the whole class (by putting your cursor on the class) or run each of the two tests individually.
@mai93 I believe the WordSpec example you posted should work now. Please take another look when you get a chance.
@srdo-humio thank you, it is working fine now! If you can resolve the other open comments and @tpasternak does not have further comments we can merge this PR.