Unify the progressbar output between jstest and jittests

RESOLVED FIXED in mozilla17

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: terrence, Assigned: terrence)

Tracking

(Blocks: 1 bug)

Trunk
mozilla17
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [js:t])

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Each test harness puts out a different set of numbers, parts of which appear to be basically useless.  The 3rd number in the jstests suite is skipped tests: a number that nobody cares about.  The last number in the jittests suite appears to be the total number of tests run: another number nobody cares about.

IMHO, the only numbers that should show up are "tests failed that should not have failed" and "tests passed that should not have passed".  This would make it very easy and intuitive to check the status of a test run: not zero == not good.

We should also print a legend for these numbers to the help text.
jit-tests also includes information about timeouts. I find this useful when running with gczeal. Lots of tests time out, but I'm really only interested in whether actual failures have taken place. It would be nice if we could preserve that.
(Assignee)

Comment 2

5 years ago
Ah, I did not know that jit-tests did that.  I think we should definitely make jstests do that as well.
(Assignee)

Updated

5 years ago
Assignee: general → nobody
Blocks: 745237
Component: JavaScript Engine → jemalloc
QA Contact: general → jemalloc
(Assignee)

Updated

5 years ago
Keywords: helpwanted
Whiteboard: [good first bug][lang=python][mentor=terrence]
Assignee: nobody → general
Component: jemalloc → JavaScript Engine
QA Contact: jemalloc → general

Comment 3

5 years ago
Links to source files, please?
(Assignee)

Comment 4

5 years ago
Given that I've marked this as a [good first bug] I should also probably say what we need here :-).  What we would like to have is:
[ nun-unexpected-fail-or-pass | num-timeout ]

This is where we update the counters that get displayed:
http://mxr.mozilla.org/mozilla-central/source/js/src/tests/results.py#115

You can update the help text by calling methods on OptionParser:
http://mxr.mozilla.org/mozilla-central/source/js/src/tests/jstests.py#55
(Assignee)

Comment 5

5 years ago
Created attachment 642814 [details] [diff] [review]
v0

I think all we want here is to add a fourth field (in the 3rd position) for timeouts.  I'm going to leave the SKIP count in the last position, because this is more use than the total count that jittest shows.
Assignee: general → terrence
Status: NEW → ASSIGNED
Attachment #642814 - Flags: review?(dmandelin)
(Assignee)

Updated

5 years ago
Blocks: 776184
(Assignee)

Updated

5 years ago
Attachment #642814 - Flags: review?(dmandelin) → review?(sphink)
Comment on attachment 642814 [details] [diff] [review]
v0

Review of attachment 642814 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/tests/lib/results.py
@@ +87,5 @@
>          self.n = 0
>  
>          self.pb = None
>          if not options.hide_progress:
> +            self.pb = ProgressBar('', testcount, 21)

I know this is picky, but any chance you could replace this magic number with something based on $COLUMNS (if set, else 80 or something)? It's fine if you just want to land it. (And no, I won't ask you to handle SIGWINCH or whatever.)

Wait! No! Never mind, this is the label width. The thing I'm talking about is currently a hardcoded constant 64 in progressbar.py. Ok, so let me change this request to use len('[%4d|%4d|%4d|%4d]' % (0,0,0,0)) instead. I just don't like the magic number.

@@ +123,2 @@
>              else:
> +                self.counts['SKIP'] += 1

Much, much nicer
Attachment #642814 - Flags: review?(sphink) → review+
(Assignee)

Comment 7

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/0b77354a5a40
(Assignee)

Updated

5 years ago
Whiteboard: [good first bug][lang=python][mentor=terrence]
Target Milestone: --- → mozilla17
(Assignee)

Updated

5 years ago
Keywords: helpwanted
(Assignee)

Updated

5 years ago
Whiteboard: [js:t]
https://hg.mozilla.org/mozilla-central/rev/0b77354a5a40
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.