Bug 59920

Created attachment 91891 [details] Patch

Attachment 91891 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/Scripts/webkitpy..." exit_code: 1 Tools/Scripts/webkitpy/tool/commands/earlywarningsystem.py:114: expected 2 blank lines, found 1 [pep8/E302] [5] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 91891 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=91891&action=review This seems OK except for the hard-coded results.html blob in your layout tests. See if you can't inject a mock somewhere instead please. :) > Tools/Scripts/webkitpy/tool/commands/earlywarningsystem.py:116 > + # FIXME: This violations abstraction englishes this not > Tools/Scripts/webkitpy/tool/commands/earlywarningsystem.py:168 > + def refetch_patch(self, patch): > + return self._tool.bugs.fetch_attachment(patch.id()) Seems shared, no? > Tools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py:142 > + tool.filesystem.write_text_file("/tmp/layout-test-results/results.html", self._example_results_html) Can't we inject a LayoutTestResults instead somewhere?

Comment on attachment 91891 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=91891&action=review >> Tools/Scripts/webkitpy/tool/commands/earlywarningsystem.py:116 >> + # FIXME: This violations abstraction > > englishes this not fixex >> Tools/Scripts/webkitpy/tool/commands/earlywarningsystem.py:168 >> + return self._tool.bugs.fetch_attachment(patch.id()) > > Seems shared, no? Yeah, but it's only one line of code. I managed to make these all one-liners. >> Tools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py:142 >> + tool.filesystem.write_text_file("/tmp/layout-test-results/results.html", self._example_results_html) > > Can't we inject a LayoutTestResults instead somewhere? Possibly. It would require this test to have a lot of knowledge of the internal structure of these objects. As is, this test is a relatively black-box integration test.

Created attachment 91893 [details] Patch

Comment on attachment 91893 [details] Patch Rejecting attachment 91893 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-8', 'build-..." exit_code: 2 Last 500 characters of output: text_file(path) File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/common/system/filesystem_mock.py", line 247, in read_text_file return self.read_binary_file(path).decode('utf-8') File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/common/system/filesystem_mock.py", line 256, in read_binary_file if self.files[path] is None: KeyError: '/mock/results.html' ---------------------------------------------------------------------- Ran 1032 tests in 28.585s FAILED (errors=1) Full output: http://queues.webkit.org/results/8531590

Created attachment 91896 [details] Archive of layout-test-results from cr-jail-8 The attached test failures were seen while running run-webkit-tests on the commit-queue. Bot: cr-jail-8 Port: Mac Platform: Mac OS X 10.6.7

Created attachment 91903 [details] Patch for landing

The commit-queue encountered the following flaky tests while processing attachment 91903 [details]: http/tests/appcache/404-manifest.html bug 59925 (author: ap@webkit.org) The commit-queue is continuing to process your patch.

Comment on attachment 91903 [details] Patch for landing Clearing flags on attachment: 91903 Committed r85469: <http://trac.webkit.org/changeset/85469>

All reviewed patches have been landed. Closing bug.

The patch broke webkitpy-tests at least on Windows, Gtk, and Qt bots. There was no python expert around, the patch had to be rolled out: https://bugs.webkit.org/show_bug.cgi?id=59928

(In reply to comment #12) > The patch broke webkitpy-tests at least on Windows, Gtk, and Qt bots. > There was no python expert around, the patch had to be rolled out: Thanks.

Committed r85504: <http://trac.webkit.org/changeset/85504>