Bug 100315 – rwt --lint-test-files doesn't handle the cascade properly

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

[Dirk Pranke]

rwt --lint-test-files doesn't handle the cascade properly

[Dirk Pranke]

Created attachment 170526 [details]
Patch

[Ojan Vafai]

Comment on attachment 170526 [details]
Patch

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

> Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:78
> +                port_to_lint.expectations_dict = lambda: {expectations_file: expectations_dict[expectations_file]}
> +                test_expectations.TestExpectations(port_to_lint, is_lint_mode=True)

This is kind of gross. It at least deserves a comment about why we're overwriting this function.

What if, we instead had TestExpectations do this looping over the expectations_dict when is_lint_mode==true? It just needs to create a new parser and parse separately for each entry, right?

[Dirk Pranke]

(In reply to comment #2)
> (From update of attachment 170526 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=170526&action=review
> 
> > Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:78
> > +                port_to_lint.expectations_dict = lambda: {expectations_file: expectations_dict[expectations_file]}
> > +                test_expectations.TestExpectations(port_to_lint, is_lint_mode=True)
> 
> This is kind of gross. It at least deserves a comment about why we're overwriting this function.
> 
> What if, we instead had TestExpectations do this looping over the expectations_dict when is_lint_mode==true? It just needs to create a new parser and parse separately for each entry, right?

It's not quite that simple; the parser itself only does syntactic checking and doesn't confirm that all the modifiers are legit or detect inter-line erors (like duplicate lines); the TestExpectations object itself does that.

That said, it wouldn't be hard to refactor the code to provide a cleaner way of doing this. I figured since the existing code was already stubbing out one function, stubbing out another might be okay, but I definitely agree that it's gross :).

[Dirk Pranke]

Created attachment 170705 [details]
push more linting logic directly into the test_expectations module

[Dirk Pranke]

Committed r132530: <http://trac.webkit.org/changeset/132530>