Bug 50205 – [chromium] new-run-webkit-tests: add better messages, return code for --lint-test-files

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Ojan Vafai]

Sync to r72870 and run "new-run-webkit-tests --lint-test-files". It should give errors that there are 3 duplicate lines on mac. It doesn't give any errors, but if you actually run the tests, it does. This means the presubmit for test_expectations isn't working.

[Dirk Pranke]

When you say "it doesn't give any errors", what are you referring to? for me, it does print messages to stderr, but the return code is 0. check-webkit-style reports the error as well. Is that what you see/saw?

[Dirk Pranke]

Created attachment 75241 [details]
Patch

[Dirk Pranke]

Per conversation w/ Ojan, it sounds like it was user error with new-run-webkit-tests and either user error with webkit-patch land-cowboy or a bug. For the former, he didn't specify --platform chromium-mac, so the command linted the platform/mac (webkit) version of the file, which was fine. 

Also, he ran land-cowboy, which skips the check-webkit-style presubmit check, which should have reported the error correctly (it does for me when I try to reproduce what he saw).

That said, I'll use this bug as an excuse to clean up the --lint-test-files code and logging. The log output should be cleaner, we no longer use the 'print' statement, and the exception raising is a bit more straightforward. Also, I removed the 'suppress_errors' keyword param to the constructor, which appears to be unused (I don't remember why it was there in the first place).

Also, in reviewing the patch you'll notice that platform/chromium was parsing the expectations files with is_lint_mode=True, and I've changed it to False. True was almost certainly the wrong value, and the new error propagation was causing things to fail in a way that the old code wouldn't (in the face of non-fatal errors).

[David Levin]

Sounds like the bug title no longer reflects what this bug is about. It would be best to fix it.

[Dirk Pranke]

good idea. done.

[Dirk Pranke]

hmm ... this seems to have slipped through the cracks. Can someone please review this?

[Dirk Pranke]

Committed r73654: <http://trac.webkit.org/changeset/73654>