Bug 86642

Summary: Add setting of additional_expectations option to chromium_unittest.test_overrides_and_builder_names()
Product: WebKit Reporter: Elliot Poger <epoger>
Component: Tools / TestsAssignee: Dirk Pranke <dpranke>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dpranke, ojan, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Elliot Poger
Reported 2012-05-16 09:42:16 PDT
Add setting of additional_expectations option to chromium_unittest.test_overrides_and_builder_names()
Attachments
Patch (3.50 KB, patch)
2012-05-16 09:45 PDT, Elliot Poger 		no flags
DetailsFormatted DiffDiff
Patch (3.55 KB, patch)
2012-05-16 12:31 PDT, Elliot Poger 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Elliot Poger
Comment 1 2012-05-16 09:45:19 PDT
Created attachment 142282 [details] Patch
Elliot Poger
Comment 2 2012-05-16 09:46:51 PDT
Dirk- can you please review this small change?
Dirk Pranke
Comment 3 2012-05-16 12:15:06 PDT
Comment on attachment 142282 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=142282&action=review looks mostly fine, just a couple of nits. > Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py:296 > + filesystem.files[chromium_overrides_path] = CHROMIUM_OVERRIDES please change this to filesystem.write_text_file(chromium_overrides_path, CHROMIUM_OVERRIDES); originally I used to allow settings .files directly, but that turns out to be fragile. > Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py:300 > + filesystem.files[additional_expectations_path] = ADDITIONAL_EXPECTATIONS same thing. Also, you probably want to use an absolute path here.
Elliot Poger
Comment 4 2012-05-16 12:30:32 PDT
Comment on attachment 142282 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=142282&action=review >> Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py:296 >> + filesystem.files[chromium_overrides_path] = CHROMIUM_OVERRIDES > > please change this to filesystem.write_text_file(chromium_overrides_path, CHROMIUM_OVERRIDES); originally I used to allow settings .files directly, but that turns out to be fragile. Done. >> Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py:300 >> + filesystem.files[additional_expectations_path] = ADDITIONAL_EXPECTATIONS > > same thing. Also, you probably want to use an absolute path here. Done.
Elliot Poger
Comment 5 2012-05-16 12:31:18 PDT
Created attachment 142324 [details] Patch
Dirk Pranke
Comment 6 2012-05-16 21:43:31 PDT
Comment on attachment 142324 [details] Patch for what it's worth, since I R+'ed the previous patch, you didn't need to set R? on this one.
Elliot Poger
Comment 7 2012-05-17 06:25:58 PDT
(In reply to comment #6) > (From update of attachment 142324 [details]) > for what it's worth, since I R+'ed the previous patch, you didn't need to set R? on this one. Unfortunately, "webkit-patch upload" set R=? for me. :-( If I didn't hear back from you, I was gonna land it manually. Thanks.
WebKit Review Bot
Comment 8 2012-05-17 06:34:25 PDT
Comment on attachment 142324 [details] Patch Clearing flags on attachment: 142324 Committed r117443: <http://trac.webkit.org/changeset/117443>
WebKit Review Bot
Comment 9 2012-05-17 06:34:29 PDT
All reviewed patches have been landed. Closing bug.
Dirk Pranke
Comment 10 2012-05-17 12:58:21 PDT
(In reply to comment #7) > (In reply to comment #6) > > (From update of attachment 142324 [details] [details]) > > for what it's worth, since I R+'ed the previous patch, you didn't need to set R? on this one. > > Unfortunately, "webkit-patch upload" set R=? for me. :-( > > If I didn't hear back from you, I was gonna land it manually. Thanks. You can actually use the --no-review flag to webkit-patch, or clear the R? manually in the bug.
Note You need to log in before you can comment on or make changes to this bug.