Bug 96393

Summary: [Chromium] Make sure that md5sum is only setup on Android.
Product: WebKit Reporter: Philippe Liard <pliard>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, adamk, dpranke, jamesr, ojan, peter, roger_fong, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 95346    
Attachments:
Description Flags
Patch
none
Patch
none
Patch abarth: review+, webkit.review.bot: commit-queue-

Philippe Liard
Reported 2012-09-11 08:23:41 PDT
Previously md5sum was setup in ChromiumAndroidDriver's constructor which is also invoked on non-Android platforms. The following patch moves this logic to _startup_test() which is guaranteed to be invoked only on Android.
Attachments
Patch (3.53 KB, patch)
2012-09-11 08:37 PDT, Philippe Liard
no flags
Patch (3.43 KB, patch)
2012-09-11 08:50 PDT, Philippe Liard
no flags
Patch (4.52 KB, patch)
2012-09-11 08:58 PDT, Philippe Liard
abarth: review+
webkit.review.bot: commit-queue-
Peter Beverloo
Comment 1 2012-09-11 08:30:06 PDT
+ the gardeners.
Philippe Liard
Comment 2 2012-09-11 08:37:18 PDT
Peter Beverloo
Comment 3 2012-09-11 08:49:15 PDT
With your patch applied, I'm now getting this assert: [1189/1616] webkitpy.layout_tests.port.chromium_android_unittest.ChromiumAndroidDriverTest.test_read_prompt failed: Traceback (most recent call last): File "/usr/local/google/build/buildbot/WebKit/Tools/Scripts/webkitpy/layout_tests/port/chromium_android_unittest.py", line 243, in test_read_prompt self.assertRaises(AssertionError, self.driver._read_prompt, time.time() + 1) AssertionError: AssertionError not raised
Philippe Liard
Comment 4 2012-09-11 08:50:12 PDT
Philippe Liard
Comment 5 2012-09-11 08:51:26 PDT
I uploaded a new patch. The previous one was incorrect. Let me see if it solves the error in your last message.
Peter Beverloo
Comment 6 2012-09-11 08:52:06 PDT
Ah, the failure is because we're accepting "$" as input now as well, so the mock input on line 242 causes that. Removing lines 242 and 243 of the unit tests should fix that, as we now accept input running as a non-root (i.e. normal) user as well, making the assert invalid.
Philippe Liard
Comment 7 2012-09-11 08:58:22 PDT
Peter Beverloo
Comment 8 2012-09-11 09:02:00 PDT
While I'm not thrilled about the name "_setup_md5sum_and_push_data_if_needed", we can address that later. I verified that the latest patch fixes the webkitpy steps.
Philippe Liard
Comment 9 2012-09-11 09:05:31 PDT
Thanks Peter and sorry for the inconvenience! I will send a CL later addressing the _setup_md5sum_and_push_data_if_needed issue.
WebKit Review Bot
Comment 10 2012-09-11 09:36:59 PDT
Comment on attachment 163378 [details] Patch Rejecting attachment 163378 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1 ERROR: /mnt/git/webkit-commit-queue/Tools/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://queues.webkit.org/results/13826288
Peter Beverloo
Comment 11 2012-09-11 09:38:01 PDT
(In reply to comment #10) > (From update of attachment 163378 [details]) > Rejecting attachment 163378 [details] from commit-queue. > > Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1 > > ERROR: /mnt/git/webkit-commit-queue/Tools/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). > > Full output: http://queues.webkit.org/results/13826288 I'll land it manually. Philippe: be sure to not remove the "Reviewed by (OOPS!)." line from your CLs :-).
Philippe Liard
Comment 12 2012-09-11 09:39:55 PDT
Ah, sorry :) Thanks for the tip and landing it.
Peter Beverloo
Comment 13 2012-09-11 09:40:24 PDT
Roger Fong
Comment 14 2012-10-02 12:49:38 PDT
Hello, I get the following error output for this test on Windows port: [439/1585] webkitpy.layout_tests.port.chromium_android_unittest.ChromiumAndroidDriverTest.test_command_from_driver_input failed: Traceback (most recent call last): File "/home/buildbot/slave/win-release-tests/build/Tools/Scripts/webkitpy/layout_tests/port/chromium_android_unittest.py", line 247, in test_command_from_driver_input self.assertEquals(self.driver._command_from_driver_input(driver_input), expected_command) AssertionError: "C:\\cygwin\\mock-checkout\\LayoutTests\\foo\\bar\\test.html'--pixel-test'checksum\n" != "/data/local/tmp/third_party/WebKit/LayoutTests/foo/bar/test.html'--pixel-test'checksum\n" Looks like it's related to this checksum stuff somehow? Thanks
Roger Fong
Comment 15 2012-10-03 11:20:05 PDT
(In reply to comment #14) > Hello, > I get the following error output for this test on Windows port: > > [439/1585] webkitpy.layout_tests.port.chromium_android_unittest.ChromiumAndroidDriverTest.test_command_from_driver_input failed: > Traceback (most recent call last): > File "/home/buildbot/slave/win-release-tests/build/Tools/Scripts/webkitpy/layout_tests/port/chromium_android_unittest.py", line 247, in test_command_from_driver_input > self.assertEquals(self.driver._command_from_driver_input(driver_input), expected_command) > AssertionError: "C:\\cygwin\\mock-checkout\\LayoutTests\\foo\\bar\\test.html'--pixel-test'checksum\n" != "/data/local/tmp/third_party/WebKit/LayoutTests/foo/bar/test.html'--pixel-test'checksum\n" > > Looks like it's related to this checksum stuff somehow? > Thanks I have not received any response. Therefore I'm going to skip this part of the test for now and reopen this bug.
Roger Fong
Comment 16 2012-10-03 11:21:04 PDT
Setting bug as unconfirmed
Roger Fong
Comment 17 2012-10-03 11:45:23 PDT
Meh, I'll just make a new bug, that makes more sense
Roger Fong
Comment 18 2012-10-03 11:54:19 PDT
(In reply to comment #17) > Meh, I'll just make a new bug, that makes more sense https://bugs.webkit.org/show_bug.cgi?id=98288
Philippe Liard
Comment 19 2012-10-04 02:13:28 PDT
Sorry guys for the late reply, I was traveling. I will look at the issue.
Roger Fong
Comment 20 2012-10-04 09:50:35 PDT
Thanks :)
Note You need to log in before you can comment on or make changes to this bug.