Bug 58588 - only show expected failure type for ports that use an expectations file
Summary: only show expected failure type for ports that use an expectations file
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Ojan Vafai
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-14 15:46 PDT by Ojan Vafai
Modified: 2011-04-14 16:31 PDT (History)
3 users (show)

See Also:


Attachments
Patch (3.28 KB, patch)
2011-04-14 15:49 PDT, Ojan Vafai
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ojan Vafai 2011-04-14 15:46:08 PDT
only show expected failure type for ports that use an expectations file
Comment 1 Ojan Vafai 2011-04-14 15:49:21 PDT
Created attachment 89673 [details]
Patch
Comment 2 Eric Seidel (no email) 2011-04-14 15:51:43 PDT
Comment on attachment 89673 [details]
Patch

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

> Tools/Scripts/webkitpy/layout_tests/layout_package/test_runner.py:159
> +    # FIXME: If non-chromium ports start using an expectations file,
> +    # we should make this check more robust.
> +    results['uses_expectations_file'] = port_obj.name().find('chromium') != -1

If... They're about to.  Sounds like dirk is working on the NRWT switch... and abarth and I are about to bring up a NRWT-using commit-queue. :)
Comment 3 Ojan Vafai 2011-04-14 15:57:49 PDT
(In reply to comment #2)
> (From update of attachment 89673 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=89673&action=review
> 
> > Tools/Scripts/webkitpy/layout_tests/layout_package/test_runner.py:159
> > +    # FIXME: If non-chromium ports start using an expectations file,
> > +    # we should make this check more robust.
> > +    results['uses_expectations_file'] = port_obj.name().find('chromium') != -1
> 
> If... They're about to.  Sounds like dirk is working on the NRWT switch... and abarth and I are about to bring up a NRWT-using commit-queue. :)

Using new-run-webkit-tests does not necessarily imply using expectations files. For now, new-run-webkit-tests uses Skipped files just fine. The discussion of whether to use expectations files is (and should be) totally orthogonal. In either case, if that changes, we can easily revert this patch.
Comment 4 Eric Seidel (no email) 2011-04-14 16:01:53 PDT
Comment on attachment 89673 [details]
Patch

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

>>> Tools/Scripts/webkitpy/layout_tests/layout_package/test_runner.py:159
>>> +    results['uses_expectations_file'] = port_obj.name().find('chromium') != -1
>> 
>> If... They're about to.  Sounds like dirk is working on the NRWT switch... and abarth and I are about to bring up a NRWT-using commit-queue. :)
> 
> Using new-run-webkit-tests does not necessarily imply using expectations files. For now, new-run-webkit-tests uses Skipped files just fine. The discussion of whether to use expectations files is (and should be) totally orthogonal. In either case, if that changes, we can easily revert this patch.

I see.  OK, I misunderstood what that was doing/and-or just didn't read carefullly. :p
Comment 5 Dirk Pranke 2011-04-14 16:09:28 PDT
I think the logic might be better/more robust if instead of guess as to whether this is a chromium/non-chromium port, or a port that does use an expectations file, you look at the actual data and see if any of the rows contain an "expected" field (or one with a value != 'PASS')?

At any rate, the apple mac NRWT port uses Skipped files plus an expectations file currently.

The patch looks fine to me otherwise.
Comment 6 Ojan Vafai 2011-04-14 16:21:13 PDT
(In reply to comment #5)
> I think the logic might be better/more robust if instead of guess as to whether this is a chromium/non-chromium port, or a port that does use an expectations file, you look at the actual data and see if any of the rows contain an "expected" field (or one with a value != 'PASS')?
> 
> At any rate, the apple mac NRWT port uses Skipped files plus an expectations file currently.
> 
> The patch looks fine to me otherwise.

Whoops. Already committed r83908: <http://trac.webkit.org/changeset/83908>. Will fix in a separate patch.
Comment 7 Ojan Vafai 2011-04-14 16:29:16 PDT
> At any rate, the apple mac NRWT port uses Skipped files plus an expectations file currently.

Actually, looking at this more, I'm not convinced it makes sense. While technically the Apple mac port uses expectations files, I'm not convinced that they will want expectations files once they actually transition to using NRWT. I have certainly heard opposition to the idea of expectations files.

I'll leave this FIXME in for now.
Comment 8 Dirk Pranke 2011-04-14 16:31:08 PDT
(In reply to comment #7)
> > At any rate, the apple mac NRWT port uses Skipped files plus an expectations file currently.
> 
> Actually, looking at this more, I'm not convinced it makes sense. While technically the Apple mac port uses expectations files, I'm not convinced that they will want expectations files once they actually transition to using NRWT. I have certainly heard opposition to the idea of expectations files.
> 
> I'll leave this FIXME in for now.

Well, I still think it might be more robust to decide what to display based on what is in the actual data rather than what you expect to be in the data (based on the port or bot name).