Bug 88947 - nrwt: implement the actual cascade of TestExpectations
Summary: nrwt: implement the actual cascade of TestExpectations
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dirk Pranke
URL:
Keywords:
Depends on: 88946
Blocks: 65834 88948
  Show dependency treegraph
 
Reported: 2012-06-12 19:43 PDT by Dirk Pranke
Modified: 2012-06-13 13:43 PDT (History)
4 users (show)

See Also:


Attachments
Patch (9.72 KB, patch)
2012-06-12 19:46 PDT, Dirk Pranke
ojan: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dirk Pranke 2012-06-12 19:43:45 PDT
nrwt: implement the actual cascade of TestExpectations
Comment 1 Dirk Pranke 2012-06-12 19:46:14 PDT
Created attachment 147218 [details]
Patch
Comment 2 Ojan Vafai 2012-06-13 09:39:13 PDT
Comment on attachment 147218 [details]
Patch

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

> Tools/ChangeLog:15
> +        There is an actual semantic change in this patch, in that
> +        setting an expectation on a directory in one file will override
> +        the expectations on any individual tests set in prior files. The
> +        test_overrides__directory() unit test verifies this.

I really think we should simplify the logic of test_expectations entirely (in a separate patch obviously) to last one wins. Even within the same expectations file. The "more specific wins" is more confusing than the benefit it brings.

> Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py:597
> +        if prev_expectation_line.filename != expectation_line.filename:

Clever!
Comment 3 Dirk Pranke 2012-06-13 10:11:09 PDT
(In reply to comment #2)
> I really think we should simplify the logic of test_expectations entirely (in a separate patch obviously) to last one wins. Even within the same expectations file. The "more specific wins" is more confusing than the benefit it brings.

Yeah, as we've discussed before, it's hard to say what the right answer is here. Once we get chromium moved to a world where long-standing expectations are checked in and the file becomes much shorter, you're probably right. In the meantime, I think allowing duplicates in the file might just make it less maintainable.

Hopefully the checked-in-failures world is not far away and we'll be able to test this theory.
Comment 4 Dirk Pranke 2012-06-13 13:43:26 PDT
Committed r120243: <http://trac.webkit.org/changeset/120243>