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

Invert status output of tests (suffix instead of prefix) #28544

Closed
slimsag opened this Issue Sep 20, 2015 · 7 comments

Comments

Projects
None yet
8 participants
@slimsag
Copy link

slimsag commented Sep 20, 2015

As it is currently running some tests looks like this:

test vec4::eq_1 ... ok
test vec4::is_zero_0 ... ok
test vec4::fmt_0 ... ok
test vec4::length_sq_0 ... ok
test vec4::is_nan_0 ... ok
test vec4::lerp_0 ... ok
test vec4::length_0 ... ok
test vec4::max_0 ... ok
test vec4::mul_0 ... ok
test vec4::mul_scalar_0 ... ok
test vec4::min_0 ... ok
test vec4::one_1 ... ok
test vec4::one_0 ... ok
test vec4::neg_0 ... ok
test vec4::partial_cmp_0 ... failed
test vec4::normalize_0 ... ok
test vec4::sub_0 ... ok
test vec4::project_0 ... ok
test vec4::round_0 ... ok
test vec4::zero_0 ... failed
test vec4::sub_scalar_0 ... ok

test result: ok. 19 passed; 2 failed; 0 ignored; 0 measured

Because the names are not the most important aspect of the message (the ok/failure status is), I believe it may make sense to invert the status output like so:

[ok] test vec4::eq_1
[ok] test vec4::is_zero_0
[ok] test vec4::fmt_0
[ok] test vec4::length_sq_0
[ok] test vec4::is_nan_0
[ok] test vec4::lerp_0
[ok] test vec4::length_0
[ok] test vec4::max_0
[ok] test vec4::mul_0
[ok] test vec4::mul_scalar_0
[ok] test vec4::min_0
[ok] test vec4::one_1
[ok] test vec4::one_0
[ok] test vec4::neg_0
[failed] test vec4::partial_cmp_0
[ok] test vec4::normalize_0
[ok] test vec4::sub_0
[ok] test vec4::project_0
[ok] test vec4::round_0
[failed] test vec4::zero_0
[ok] test vec4::sub_scalar_0

test result: ok. 19 passed; 2 failed; 0 ignored; 0 measured

Note that:

  1. This makes it easier to identify which tests failed or not, because the status is not offset by a name of arbitrary length.
  2. This might not make sense if printing the test name is important prior to the status (e.g. if the output is not fully buffered, or if test programs may sometimes hang and the testing framework can't catch this).

Moved here from rust-lang/cargo#1985

@christopherdumas

This comment has been minimized.

Copy link
Contributor

christopherdumas commented Sep 20, 2015

👍 I agree, and I'd be willing to implement this.

@steveklabnik

This comment has been minimized.

Copy link
Member

steveklabnik commented Sep 28, 2015

👍

@viraptor

This comment has been minimized.

Copy link

viraptor commented Nov 26, 2015

This will be pretty bad for accidental infinite loops. It's already required to turn off multithreaded testing to figure out which test is even hanging. Unless there's also some way to add test timeouts by default, switching the output order will be a pain.

@brson

This comment has been minimized.

Copy link
Contributor

brson commented May 4, 2017

Seems like a worthy idea. Let's see if the tools team wants to consider it.

@nagisa

This comment has been minimized.

Copy link
Contributor

nagisa commented May 4, 2017

The name can be printed “before” status (in time as opposed to space) by using some escape sequence tricks to reset the cursor to the original position (i.e. in the similar manner progress bars are implemented)

@viraptor

This comment has been minimized.

Copy link

viraptor commented May 4, 2017

Is this still a problem, now that cargo test uses green / red colours in the output? (if I understand correctly, the original feature was about easy visibility)

@alexcrichton alexcrichton added T-dev-tools and removed T-tools labels May 22, 2017

@nrc

This comment has been minimized.

Copy link
Member

nrc commented May 22, 2017

We discussed this at the dev-tools meeting. Our verdict was that this should remain P-low. There was some discussion about printing the test name before a test is run (this is useful if a test segfaults, then you know which test is to blame) - which is #30047.

On this issue, we decided we should not change things because there is low benefit (now that ok/fail is coloured) and there is always a cost to change.

@nrc nrc closed this May 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.