Bug 59920

Summary: Add base case for a test-running EWS
Product: WebKit Reporter: Adam Barth <abarth>
Component: New BugsAssignee: Adam Barth <abarth>
Status: RESOLVED FIXED    
Severity: Normal CC: abecsi, commit-queue, eric, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Bug Depends on: 59928    
Bug Blocks: 59272    
Attachments:
Description Flags
Patch
none
Patch
none
Archive of layout-test-results from cr-jail-8
none
Patch for landing none

Description Adam Barth 2011-05-02 00:36:56 PDT
Add base case for a test-running EWS
Comment 1 Adam Barth 2011-05-02 00:37:38 PDT
Created attachment 91891 [details]
Patch
Comment 2 WebKit Review Bot 2011-05-02 00:39:50 PDT
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 3 Eric Seidel (no email) 2011-05-02 00:44:38 PDT
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 4 Adam Barth 2011-05-02 00:48:21 PDT
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.
Comment 5 Adam Barth 2011-05-02 00:49:20 PDT
Created attachment 91893 [details]
Patch
Comment 6 WebKit Commit Bot 2011-05-02 01:18:35 PDT
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
Comment 7 WebKit Commit Bot 2011-05-02 01:18:39 PDT
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
Comment 8 Adam Barth 2011-05-02 03:10:58 PDT
Created attachment 91903 [details]
Patch for landing
Comment 9 WebKit Commit Bot 2011-05-02 03:50:56 PDT
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 10 WebKit Commit Bot 2011-05-02 03:52:02 PDT
Comment on attachment 91903 [details]
Patch for landing

Clearing flags on attachment: 91903

Committed r85469: <http://trac.webkit.org/changeset/85469>
Comment 11 WebKit Commit Bot 2011-05-02 03:52:08 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Andras Becsi 2011-05-02 05:20:27 PDT
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
Comment 13 Adam Barth 2011-05-02 12:17:55 PDT
(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.
Comment 14 Adam Barth 2011-05-02 12:42:32 PDT
Committed r85504: <http://trac.webkit.org/changeset/85504>