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

Testing toolchain and Junit deps #1102

Merged
merged 3 commits into from
Sep 19, 2020
Merged

Conversation

liucijus
Copy link
Collaborator

This PR is part of refactoring towards toolchains #940

This is a proposal for a generic testing toolchain, which can be used to configure multiple testing libraries. Some users may want to use multiple testing libraries or implementations that depend on Junit. Instead of setting up multiple toolchains (which look very similar), I think it it is more practical to have a single toolchain type, which allows to provide dependencies for mutiple testing libraries.

This PR covers toolchain itself and configuration for JUnit.

Support for ScalaTest and Specs2 will be developed as separate PRs.

@ittaiz
Copy link
Member

ittaiz commented Sep 11, 2020

Skimmed it.
Is the end goal to have in the generic testing toolchain junit + specs2 + scalatest?
If so what do we gain out of this coupling?
If it's not the end goal can you explain how will it look like for scalatest?
Also looks a bit related (not completely) to #959

@liucijus
Copy link
Collaborator Author

My goal is simplicity, I think there's very little gain with having a separate toolchain per test library. From user point of view it's much easier to set up it for cases like specs2 with junit (instead of two toolchains, only one is reaquired). And this is step towards generic testing harness.

@ittaiz
Copy link
Member

ittaiz commented Sep 11, 2020

I really feel that trying together scalatest with junit goes agaisnt the bazel idioms of separation.
Why should a scalatest user need junit?
junit+specs2 is the exception to the rule and not the rule IMHO

@liucijus
Copy link
Collaborator Author

The only thing which is shared is the toolchain type, scalatest rule won't know anything about junit.

@olafurpg
Copy link
Contributor

Why should a scalatest user need junit?

At Twitter, we're looking into using scala_junit_test to run ScalaTest + JUnit tests together. We have implemented a custom RunnerBuilder that infers the @RunWith(classOf[JUnitRunner]) annotation for all subclasses of org.scalatest.Suite. The benefit of this approach is that we have a unified test runner for Java and Scala (consistent test filtering, report formatting, ...). This approach is also similar to how the current Pants test runner works, and can be extended to support other JUnit-compatible testing libraries like MUnit or zio-test-junit.

@ittaiz
Copy link
Member

ittaiz commented Sep 11, 2020 via email

@@ -31,3 +31,5 @@ def junit_repositories(maven_servers = _default_maven_server_urls()):
name = "io_bazel_rules_scala/dependency/hamcrest/hamcrest_core",
actual = "@io_bazel_rules_scala_org_hamcrest_hamcrest_core//jar",
)

native.register_toolchains("@io_bazel_rules_scala//testing:testing_toolchain")
Copy link
Member

Choose a reason for hiding this comment

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

This is the default? For people who don't override?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, but probably it will stay here only until Specs2 and ScalaTest will be migrated. Each test lib will have it's own configuration.

@@ -0,0 +1,45 @@
## Testing toolchain configuration
Copy link
Member

Choose a reason for hiding this comment

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

The idea is that Wix can set up a testing toolchain of junit and specs2, twitter of junit and scalatest and stripe only if scalatest?
Since unneeded rules won't be evaluated and the missing deps won't be a problem? If so it will probably be worthwhile to make sure that bazel toolchains actually works like that (for example move scalatest to this and see that Wix defines only specs2 + junit in the single testing toolchain and everything works).
Finally I think it's important in clarifying this idea.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Toolchain only knows a list of labels, whether it correctly works depends on the actual rule that uses this toolchain and requests any of the dependencies. So if there is no rule that asks for JUnit, there's no need to add Junit deps to the toolchain.
Docs will be updated when there will be more rules that use it.

Copy link
Member

Choose a reason for hiding this comment

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

I think there's value in explicitly specifying this issue of actual deps.
Most of our users will not understand toolchain in the same depths you do.

@ittaiz ittaiz merged commit f0c8d07 into bazelbuild:master Sep 19, 2020
blorente pushed a commit to twitter-forks/rules_scala that referenced this pull request Nov 26, 2020
* Add testing toolchain with Junit deps

* Add docs for junit

* Update junit related aspect test deps
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants