RESOLVED FIXED 50205
[chromium] new-run-webkit-tests: add better messages, return code for --lint-test-files 
https://bugs.webkit.org/show_bug.cgi?id=50205
Summary [chromium] new-run-webkit-tests: add better messages, return code for --lint-...
Ojan Vafai
Reported 2010-11-29 20:19:03 PST
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.
Attachments
Patch (20.79 KB, patch)
2010-11-30 18:55 PST, Dirk Pranke 		ojan: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Dirk Pranke
Comment 1 2010-11-30 16:38:47 PST
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
Comment 2 2010-11-30 18:55:54 PST
Created attachment 75241 [details] Patch
Dirk Pranke
Comment 3 2010-11-30 19:00:50 PST
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
Comment 4 2010-12-03 07:01:37 PST
Sounds like the bug title no longer reflects what this bug is about. It would be best to fix it.
Dirk Pranke
Comment 5 2010-12-03 11:41:01 PST
good idea. done.
Dirk Pranke
Comment 6 2010-12-08 20:29:29 PST
hmm ... this seems to have slipped through the cracks. Can someone please review this?
Dirk Pranke
Comment 7 2010-12-09 15:22:25 PST
Committed r73654: <http://trac.webkit.org/changeset/73654>
Note You need to log in before you can comment on or make changes to this bug.