rules_scala icon indicating copy to clipboard operation
rules_scala copied to clipboard

Generic scala_test rule

Open andyscott opened this issue 5 years ago • 15 comments

Description

Adds a new scala_test rule that supports the SBT testing interface.

Initially I would like to have the rule merged as "experimental" or "unstable" so folks can begin using it in larger codebase and we can iterate and improve upon the implementation.

Here's some sample output where you can see ScalaTest, Scalacheck, and JUnit all running in the same target:

 bazel test //test/v2/... --sandbox_debug=true --test_output=all                                                                                      (sc:no-domain)
DEBUG: /Users/andyscott/personal/git/bazelbuild/rules_scala/scala/private/phases/phase_discover_tests.bzl:2:5: discover tests
INFO: Analyzed 3 targets (0 packages loaded, 0 targets configured).
INFO: Found 2 targets and 1 test target...
INFO: From Testing //test/v2:test:
==================== Test output for //test/v2:test:

> beginning run of com.novocode.junit.JUnitFramework
debug: Test run started
debug: Test test.v2.JUnitTest.testFoo started
debug: Test test.v2.JUnitTest.testFoo finished, took 0.001 sec
debug: Test run finished: 0 failed, 0 ignored, 1 total, 0.004s

< run of com.novocode.junit.JUnitFramework complete

> beginning run of org.scalacheck.ScalaCheckFramework
info: + TestPropertiesClass.1: OK, proved property.
info: + TestPropertiesObject.2: OK, proved property.
Passed: Total 2, Failed 0, Errors 0, Passed 2
< run of org.scalacheck.ScalaCheckFramework complete

> beginning run of org.scalatest.tools.Framework
info: TestSuiteClass:
info: - method1
info: - method2
Run completed in 97 milliseconds.
Total number of tests run: 2
Suites: completed 1, aborted 0
Tests: succeeded 2, failed 0, canceled 0, ignored 0, pending 0
All tests passed.
< run of org.scalatest.tools.Framework complete
================================================================================
INFO: Elapsed time: 2.123s, Critical Path: 2.00s
INFO: 3 processes: 2 darwin-sandbox, 1 worker.
INFO: Build completed successfully, 3 total actions
//test/v2:test                                                           PASSED in 1.5s

Executed 1 out of 1 test: 1 test passes.
INFO: Build completed successfully, 3 total actions

I'll add a detailed description once I get this working.

Note: This branch contains a lot of refactoring and cleanup that I've encountered while building the new rule. These changes have been split off into other PRs. Depending on their merge status, they may or may not be bloating the diffs here.

  • https://github.com/bazelbuild/rules_scala/pull/955 unstable set of rules
  • https://github.com/bazelbuild/rules_scala/pull/956 use worker proto file
  • https://github.com/bazelbuild/rules_scala/pull/957 ignore metals directory
  • https://github.com/bazelbuild/rules_scala/pull/958 unify default info creation

Motivation

Make scala_test support any testing library that implements the SBT testing interface. Ideally we can get rid of the three separate test rules and three corresponding test runners we currently have and use something generic like this.

andyscott avatar Jan 27 '20 04:01 andyscott

I'll take a look once we finalize the other PRs, ok?

ittaiz avatar Jan 27 '20 05:01 ittaiz

This PR will address #951 (If I understand it correctly). Lucid is +1 for this change, we use specs2 for all Scala tests but scala_test currently doesn't support it. I notice scala_specs2_junit_test is designed for this purpose but it would be better to just have one unified scala_test. Plus, scala_junit_test and scala_test are quite similar (based on what phases are used), another good reason to support this PR.

Let's wait until all the split off PRs are merged. @ittaiz what are the PRs you are waiting on to finalize?

borkaehw avatar Feb 20 '20 19:02 borkaehw

for one- #1005 Additionally @andyscott needs to rebase and say if this is ready for review (note the WIP header). After that's done I'll try to get the review started within a few days. I'll remind @andyscott that I'm waiting for his data collection over at #965 which I'd like to advance before other issues. I'm thinking of just moving to providers without it but he said he'll collect the data in a few weeks so I'm waiting

ittaiz avatar Feb 21 '20 14:02 ittaiz

I'll remind @andyscott that I'm waiting for his data collection over at #965 which I'd like to advance before other issues. I'm thinking of just moving to providers without it but he said he'll collect the data in a few weeks so I'm waiting

It will happen :). I've been busy! I'd like to get this PR sorted out and then use any future time for reviews and shepherding through work by other folks.

andyscott avatar Feb 21 '20 17:02 andyscott

It will happen :)

👍 Once we merge #1005, you rebase this PR and say that it's ready I'll see when I can make time to review it.

Just a few thoughts- How does test discovery happen? How does IDE integration happen? How does test filtering happen?

ittaiz avatar Feb 22 '20 08:02 ittaiz

Status?

ghost avatar May 11 '20 00:05 ghost

Good question. @andyscott can you rebase and answer my questions above?

ittaiz avatar May 11 '20 02:05 ittaiz

Sure thing, I'll go through this tomorrow.

On Sun, May 10, 2020, 19:45 Ittai Zeidman [email protected] wrote:

Good question. @andyscott https://github.com/andyscott can you rebase and answer my questions above?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bazelbuild/rules_scala/pull/959#issuecomment-626441256, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACLYW7IIIOOIE4J7EJKTETRQ5RE3ANCNFSM4KL3OBHA .

andyscott avatar May 11 '20 05:05 andyscott

Any news?

darl avatar May 29 '20 10:05 darl

I should have this cleaned up today. Regarding your questions @ittaiz:

How does test discovery happen?

Test discovery is done by scanning the classpath and identifying classes/modules that satisfy the SBT testing interface.

How does IDE integration happen?

I'm not sure, I haven't tried it.

How does test filtering happen?

I haven't explored extending this for test filtering, so I'm not sure.

I don't have time to work on IDE integration or test filtering. I suggest we do one of two things:

  1. Merge the PR without testing IDE support and without test filtering. Other folks from the community can pick up these work items.
  2. Someone else continues on this PR to add the additional functionality before it gets merged.

I'm biased towards option 1, as the functionality is marked unstable/experimental and it gives closure to this PR.

andyscott avatar May 29 '20 17:05 andyscott

I'll take a look this weekend and see what we can do. Can you get the build to pass?

ittaiz avatar May 31 '20 19:05 ittaiz

Yep. I need to do a few more fixes, and then there's some obvious cleanup and refactoring needed too.

On Sun, May 31, 2020, 12:05 Ittai Zeidman [email protected] wrote:

I'll take a look this weekend and see what we can do. Can you get the build to pass?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bazelbuild/rules_scala/pull/959#issuecomment-636514051, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACLYW2RUGJIAQXLECLCEDDRUKS55ANCNFSM4KL3OBHA .

andyscott avatar May 31 '20 19:05 andyscott

Ok. LMK when to look.

On Sun, 31 May 2020 at 22:49 Andy Scott [email protected] wrote:

Yep. I need to do a few more fixes, and then there's some obvious cleanup and refactoring needed too.

On Sun, May 31, 2020, 12:05 Ittai Zeidman [email protected] wrote:

I'll take a look this weekend and see what we can do. Can you get the build to pass?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub < https://github.com/bazelbuild/rules_scala/pull/959#issuecomment-636514051 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AACLYW2RUGJIAQXLECLCEDDRUKS55ANCNFSM4KL3OBHA

.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/bazelbuild/rules_scala/pull/959#issuecomment-636519672, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKQQF6AYKJ6ZBW7G7WFBALRUKYCRANCNFSM4KL3OBHA .

--

Ittai Zeidman

40 Hanamal street, Tel Aviv, Israel

http://www.wix.com

ittaiz avatar Jun 01 '20 02:06 ittaiz

We're getting closer!

if you have time, it might be good to begin reading through the code to provide feedback on organization. Separately, I am going to read through my own implementation (it's been a few months since I've dug into the Scala part of this) and add comments.

andyscott avatar Jun 01 '20 23:06 andyscott

I’ll make time over the weekend, really swamped until then

On Tue, 2 Jun 2020 at 2:21 Andy Scott [email protected] wrote:

We're getting closer!

if you have time, it might be good to begin reading through the code to provide feedback on organization. Separately, I am going to read through my own implementation (it's been a few months since I've dug into the Scala part of this) and add comments.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bazelbuild/rules_scala/pull/959#issuecomment-637176865, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKQQF33JP2XWIWDSJAB5R3RUQZYXANCNFSM4KL3OBHA .

--

Ittai Zeidman

40 Hanamal street, Tel Aviv, Israel

http://www.wix.com

ittaiz avatar Jun 02 '20 04:06 ittaiz

closing as stale, feel free to reopen

liucijus avatar Jan 31 '24 15:01 liucijus