-
Notifications
You must be signed in to change notification settings - Fork 277
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
thoughts on colorful test output #255
Comments
I think having an option for scala_test for this (and short stacks for
instance) would be a great idea.
…On Wed, Jul 26, 2017 at 15:11 doug tangren ***@***.***> wrote:
At the moment it seems likely any streamed test output is hardcoded
<https://github.com/bazelbuild/rules_scala/blob/master/scala/scala.bzl#L685>
to suppress color ( -W means without color according to the Runner docs
<http://www.scalatest.org/user_guide/using_the_runner>. For engineers
that are coming from sbt and onto baze, color is something that is missed.
It also holds practical value to have test failures visually jump out at
you in red. What are your thoughts on a patch that would make color and
option available to end users?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#255>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAEJdoQPUlxbZcDKPtNMxBAk5PATXovsks5sR49vgaJpZM4OkXEW>
.
|
Good to hear. I was just taking the temperature before I sketched out a branch. I may not get to this immediately but will likely circle back at some point. |
Doesn't sound like specs2 and JUnit are a priority for you but I'll just
mention I hope to dumb it down to the macro level of calling scala library
and java_test. Need to see when I'll get to it
…On Wed, 26 Jul 2017 at 22:29 doug tangren ***@***.***> wrote:
Good to hear. I was just taking the temperature before I sketched out a
branch. I may not get to this immediately but will likely circle back at
some point.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#255 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABUIFyM2qg7VBC2wH8ECDpuduJWBWWr1ks5sR5OxgaJpZM4OkXEW>
.
|
I think the two, or three different test runners can work this in as separate efforts for the different rule interfaces. We mainly only use scala_test so that's the one I'd likely be targeting. Our engineers really like its colored output in sbt and we're trying to make the transition a pleasant one. |
I've got a branch for this but am unsure what a good test for this looks like. would it be okay for open a prelim or to get some initial feedback on what a test might look like? |
Definitely.
Though I have to warn you that I'm working on a big diff with moving from
manual javac to java_common.compile and adding `strict java deps` support
to java.
…On Sat, Aug 26, 2017 at 11:52 PM doug tangren ***@***.***> wrote:
I've got a branch for this but am unsure what a good test for this looks
like. would it be okay for open a prelim or to get some initial feedback on
what a test might look like?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#255 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABUIF5PIhk7nprWmHDEhGsAw8-xdpUxKks5scIWqgaJpZM4OkXEW>
.
|
Rendered #268 |
@ittaiz I'm not a big fan of big diffs. Can we possibly factor your big diff into chunks each <= 400 lines ideally? In my experience, review quality goes way down on big PRs and sunk cost fallacy starts to build pressure to just merge what is on hand. |
I agree. I've split the change to small logical commits which are
reviewable.
The biggest one and the one containing 80% of the logic is 75 lines
addition and about tha in deletions.
It's big because it touches all existing code paths and reasoning about it
in our god file bzl is hard.
We want to do a major restructuring of it to break apart the big ball of
mud but we're currently tied up. Of course brainstorm the solution together.
…On Mon, 28 Aug 2017 at 2:56 P. Oscar Boykin ***@***.***> wrote:
@ittaiz <https://github.com/ittaiz> I'm not a big fan of big diffs. Can
we possibly factor your big diff into chunks each <= 400 lines ideally? In
my experience, review quality goes way down on big PRs and sunk cost
fallacy starts to build pressure to just merge what is on hand.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#255 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABUIF-C_6Q3Sd8MPlatolMa5VdFKz1Vqks5scgIWgaJpZM4OkXEW>
.
|
Just looking to guesstimate if I can factor in a upgrade of rules scala in an upcoming sprint at work. For this particular change, it's pretty small but wasn't sure what good tests would look like. I'm happy to wait on a larger pull to make its way through but do we have an eta on when that would happen? On my end, my work task just looks like "improve the bazel testing experience". I think I can accomplish alot with few lines of change but I want to stick as close to head of rules scala as I can. |
I think I'll open the PR today BUT:
1. I suspect there are two missing bits (I might have broken IntelliJ
support and some meta aspects)
2. I'm on vacation Tuesday-Friday
Given both of the above I estimate this will be merged Monday next week
…On Mon, 28 Aug 2017 at 8:57 doug tangren ***@***.***> wrote:
Just looking to guesstimate if I can factor in a upgrade of rules scala in
an upcoming sprint at work. For this particular change, it's pretty small
but wasn't sure what good tests would look like. I'm happy to wait on a
larger pull to make its way through but do we have an eta on when that
would happen? On my end, my work task just looks like "improve the bazel
testing experience". I think I can accomplish alot with few lines of change
but I want to stick as close to head of rules scala as I can.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#255 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABUIF3vu6UkxlXh-mpgPwC3F42kBOKh4ks5sclbdgaJpZM4OkXEW>
.
|
Can we close this out now? |
Case closed! |
At the moment it seems likely any streamed test output is hardcoded to suppress color ( -W means without color according to the Runner docs ). For engineers that are coming from sbt and onto baze, color is something that is missed. It also holds practical value to have test failures visually jump out at you in red. What are your thoughts on a patch that would make color and option available to end users?
The text was updated successfully, but these errors were encountered: