Bug 64215 - NRWT results.html page should show a count of the number of tests in each category
Summary: NRWT results.html page should show a count of the number of tests in each cat...
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: Nobody
URL:
Keywords:
Depends on:
Blocks: 64491
  Show dependency treegraph
 
Reported: 2011-07-08 16:27 PDT by Simon Fraser (smfr)
Modified: 2012-05-22 04:43 PDT (History)
9 users (show)

See Also:


Attachments
proposed fix (1.82 KB, patch)
2012-05-08 08:46 PDT, Kristóf Kosztyó
ojan: review-
ojan: commit-queue-
Details | Formatted Diff | Diff
proposed fix with unit test (3.78 KB, patch)
2012-05-21 01:18 PDT, Kristóf Kosztyó
ojan: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-02 (597.63 KB, application/zip)
2012-05-21 03:09 PDT, WebKit Review Bot
no flags Details
proposed fix with unit test (7.34 KB, patch)
2012-05-22 01:26 PDT, Kristóf Kosztyó
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2011-07-08 16:27:23 PDT
The "Tests where results did not match expected results" text should show a count of the number of tests that failed, so I can easily compare two sets of results and see if the counts differ.
Comment 1 Kristóf Kosztyó 2012-05-08 08:46:06 PDT
Created attachment 140723 [details]
proposed fix
Comment 2 Ojan Vafai 2012-05-08 15:20:43 PDT
Comment on attachment 140723 [details]
proposed fix

What's the use-case for this? I'm not opposed to adding this if there's a good benefit, but I'm sensitive to the clutter/usefulness tradeoff.

Also, this will need a test. See http://trac.webkit.org/browser/trunk/LayoutTests/fast/harness/resources/results-test.js.
Comment 3 Simon Fraser (smfr) 2012-05-08 15:22:58 PDT
I want this because it provides an "at a glance" way to know if you broke more tests than were already broken when making a change.
Comment 4 Ojan Vafai 2012-05-08 15:24:56 PDT
(In reply to comment #3)
> I want this because it provides an "at a glance" way to know if you broke more tests than were already broken when making a change.

I see. That's fine with me. This patch just needs a test then. The existing test may actually cover this, but will need a new expected result. It just runs with run-webkit-tests since it's in the LayoutTests directory.
Comment 5 Kristóf Kosztyó 2012-05-09 12:55:12 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > I want this because it provides an "at a glance" way to know if you broke more tests than were already broken when making a change.
> 
> I see. That's fine with me. This patch just needs a test then. The existing test may actually cover this, but will need a new expected result. It just runs with run-webkit-tests since it's in the LayoutTests directory.

Hi,
I ran the nrwt for this test, and it didn't fail. It seems the existing test doesn't cover this hence I've looked the results-test.js how I could write a test for it, but I've lost in the code :(
Could you explain how I should do that?
Comment 6 Ojan Vafai 2012-05-09 20:15:21 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > I want this because it provides an "at a glance" way to know if you broke more tests than were already broken when making a change.
> > 
> > I see. That's fine with me. This patch just needs a test then. The existing test may actually cover this, but will need a new expected result. It just runs with run-webkit-tests since it's in the LayoutTests directory.
> 
> Hi,
> I ran the nrwt for this test, and it didn't fail. It seems the existing test doesn't cover this hence I've looked the results-test.js how I could write a test for it, but I've lost in the code :(
> Could you explain how I should do that?

Take any of the existing runTest calls and add the appropriate assertions that the number of failing tests matches the number of failing tests in the mockResults object. Make sure to cover the testList case and the failingTestsTable case.
Comment 7 Kristóf Kosztyó 2012-05-21 01:18:51 PDT
Created attachment 142963 [details]
proposed fix with unit test
Comment 8 WebKit Review Bot 2012-05-21 03:09:48 PDT
Comment on attachment 142963 [details]
proposed fix with unit test

Attachment 142963 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12744154

New failing tests:
fast/harness/results.html
Comment 9 WebKit Review Bot 2012-05-21 03:09:53 PDT
Created attachment 142980 [details]
Archive of layout-test-results from ec2-cr-linux-02

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-02  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment 10 Kristóf Kosztyó 2012-05-21 04:41:36 PDT
(In reply to comment #8)
> (From update of attachment 142963 [details])
> Attachment 142963 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/12744154
> 
> New failing tests:
> fast/harness/results.html

Interesting. I've updated the expectation for this test and it runs fine locally.
Comment 11 Ojan Vafai 2012-05-21 10:26:20 PDT
(In reply to comment #10)
> (In reply to comment #8)
> > (From update of attachment 142963 [details] [details])
> > Attachment 142963 [details] [details] did not pass chromium-ews (chromium-xvfb):
> > Output: http://queues.webkit.org/results/12744154
> > 
> > New failing tests:
> > fast/harness/results.html
> 
> Interesting. I've updated the expectation for this test and it runs fine locally.

The problem is that there's also a results-expected.txt in platform/win and platform/chromium-win. I'm 99% sure that you can just delete the platform/chromium-win result now that chromium-win no longer falls back to platform/win, but you'll also need to update the platform/win result to keep the test passing on Apple's Win port.
Comment 12 Kristóf Kosztyó 2012-05-22 01:26:26 PDT
Created attachment 143220 [details]
proposed fix with unit test

I looked to platform/win for update the expectation but it is really old (2011-10-20).
The apple's win bots have "dead" for a long time so I couldn't update it.
If the ews doesn't go red I will cq+ it.
Comment 13 WebKit Review Bot 2012-05-22 04:43:11 PDT
Comment on attachment 143220 [details]
proposed fix with unit test

Clearing flags on attachment: 143220

Committed r117950: <http://trac.webkit.org/changeset/117950>
Comment 14 WebKit Review Bot 2012-05-22 04:43:16 PDT
All reviewed patches have been landed.  Closing bug.