Bug 106562 - NRWT still confused about test count with --repeat-each and --iterations
Summary: NRWT still confused about test count with --repeat-each and --iterations
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jussi Kukkonen (jku)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-10 07:10 PST by Jussi Kukkonen (jku)
Modified: 2013-01-16 13:42 PST (History)
6 users (show)

See Also:


Attachments
Patch (1.73 KB, patch)
2013-01-10 07:17 PST, Jussi Kukkonen (jku)
no flags Details | Formatted Diff | Diff
remove the useless variable as well (2.07 KB, patch)
2013-01-10 07:30 PST, Jussi Kukkonen (jku)
no flags Details | Formatted Diff | Diff
Add test for print_found() (3.44 KB, patch)
2013-01-15 13:01 PST, Jussi Kukkonen (jku)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jussi Kukkonen (jku) 2013-01-10 07:10:44 PST
A reference call to r-w-t with no "--iterations" or "--repeat-each" results in these calculations:
 Found 9859 tests; running 9015, skipping 844.

And the same call with "--repeat-each=2 --iterations=2"
 Found 9859 tests; running 2253 (4 times each: --repeat-each=2 --iterations=2), skipping 7606.

That is quite wrong, I'll attach a patch.
Comment 1 Jussi Kukkonen (jku) 2013-01-10 07:17:07 PST
Created attachment 182132 [details]
Patch
Comment 2 Jussi Kukkonen (jku) 2013-01-10 07:20:50 PST
Adding dpranke and ojan (from bug 95789), maybe either of you wants to review?
Comment 3 Zan Dobersek 2013-01-10 07:22:40 PST
Comment on attachment 182132 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=182132&action=review

> Tools/Scripts/webkitpy/layout_tests/views/printing.py:101
> +        num_unique_tests = num_to_run

Assuming that this works, you can just use num_to_run wherever num_unique_tests is used.
Comment 4 Jussi Kukkonen (jku) 2013-01-10 07:30:31 PST
Created attachment 182136 [details]
remove the useless variable as well
Comment 5 Dirk Pranke 2013-01-14 17:28:17 PST
Comment on attachment 182136 [details]
remove the useless variable as well

Okay, so it appears that this patch changes things so that we print the # of distinct tests we found, the number of those that we'll actually run, the number of unique tests that we'll actually skip, and the --repeat-each and --iterations values, right?

So, we're not printing the aggregate totals, i.e., if we're running 100 tests ten times each, we'll never print 1000.

Did your change break any tests? It sure seems like it should've. If it doesn't, that means that this isn't being tested, and since it's behavior we've probably broken at least twice, we should probably have a test for it (even though this is just a cosmetic thing). Mind adding a test?
Comment 6 Jussi Kukkonen (jku) 2013-01-15 13:01:01 PST
Created attachment 182833 [details]
Add test for print_found()
Comment 7 Dirk Pranke 2013-01-15 13:06:29 PST
Comment on attachment 182833 [details]
Add test for print_found()

looks great, thanks!
Comment 8 WebKit Review Bot 2013-01-16 00:03:57 PST
Comment on attachment 182833 [details]
Add test for print_found()

Clearing flags on attachment: 182833

Committed r139841: <http://trac.webkit.org/changeset/139841>
Comment 9 WebKit Review Bot 2013-01-16 00:04:02 PST
All reviewed patches have been landed.  Closing bug.
Comment 10 Dirk Pranke 2013-01-16 12:59:35 PST
Comment on attachment 182833 [details]
Add test for print_found()

View in context: https://bugs.webkit.org/attachment.cgi?id=182833&action=review

> Tools/Scripts/webkitpy/layout_tests/views/printing.py:106
> +        print (found_str)

turns out this got left in accidentally and I missed it in the review. Fixed in http://trac.webkit.org/changeset/139913 .
Comment 11 Jussi Kukkonen (jku) 2013-01-16 13:42:24 PST
(In reply to comment #10)
> turns out this got left in accidentally and I missed it in the review. Fixed in http://trac.webkit.org/changeset/139913 .

Thanks for handling it.