Bug 58588 – only show expected failure type for ports that use an expectations file

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Ojan Vafai]

only show expected failure type for ports that use an expectations file

[Ojan Vafai]

Created attachment 89673 [details]
Patch

[Eric Seidel (no email)]

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. :)

[Ojan Vafai]

(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.

[Eric Seidel (no email)]

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

[Dirk Pranke]

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.

[Ojan Vafai]

(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.

[Ojan Vafai]

> 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.

[Dirk Pranke]

(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).