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

Factor test toolchain #984

Closed
wants to merge 27 commits into from

Conversation

smparkes
Copy link
Contributor

@smparkes smparkes commented Jan 31, 2020

Please look at #962 and consider earlier uncommitted PRs first. This PR is based on those PRs and will give you smaller PRs to review.

Description

  • Create a new test toolchain with a provider in the normal pattern
  • Move scalatest jars/dependencies to the toolchain
  • Move scalatest flags to the toolchain
  • Tiny bit of cleanup (don't "_" prefix in BUILD files since they can't be loaded)

Motivation

Multiple scala version builds within the same bazel run #962

@andyscott
Copy link
Contributor

andyscott commented Jan 31, 2020

This looks super neat. A few comments:

  • can we rename bootstrap_toolchain to scala_toolchain?
  • can we rename test_toolchain to scala_test_toolchain?
  • can we update the jar field to instead be deps and accept multiple JavaInfo targets? This wills likely scale better. In particular I can imagine pushing this a bit further to work well with Generic scala_test rule #959 for easily configuring test framework deps.

@smparkes
Copy link
Contributor Author

@andyscott Think you may want to look at #978 first. More reviewable since it's a smaller chunk. I'll respond to some of the comments there since they're in the earlier PRs.

can we rename test_toolchain to scala_test_toolchain?

I've been wondering about this. It's been unclear to me how much this is any scala test or scalatest. We obviously provide the scalatest (and scalactic? ... haven't gotten there yet ...) jars automatically.

In #959, is the idea still fundamentally scalatest with extensions or is it "a scala test" for some framework ...

can we update the jar field to instead be deps and accept multiple JavaInfo targets?

Yup.

@andyscott
Copy link
Contributor

#959 is a scala_test rule that behaves like SBT's test command: it runs whatever tests can be discovered by any testing frameworks on the classpath. It's not specific to Scalatest, Specs2, uTest, Minitest, etc.

Coupled with some cleanup to toolchains it would be possible to configure the generic #959 scala_test rule to behave exactly like the current scala_test rule. The toolchain would configure the Scalatest framework jars and then the test rule would only run Scalatest tests. A user, of course, could configure Scalacheck and Scalatest jars, and then the test rules would discover tests from both Scalacheck and Scalatest.

@smparkes smparkes mentioned this pull request Jan 31, 2020
@smparkes
Copy link
Contributor Author

Updated the toolchain name and the jar -> deps thing.

The other naming is pending ...

@smparkes
Copy link
Contributor Author

At this point, there's not enough need/support for this (including myself) to continue forward.

@smparkes smparkes closed this May 14, 2020
gergelyfabian pushed a commit to gergelyfabian/rules_scala that referenced this pull request May 31, 2022
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.

3 participants