Bug 96393 - [Chromium] Make sure that md5sum is only setup on Android.
Summary: [Chromium] Make sure that md5sum is only setup on Android.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 95346
  Show dependency treegraph
 
Reported: 2012-09-11 08:23 PDT by Philippe Liard
Modified: 2012-10-04 09:50 PDT (History)
8 users (show)

See Also:


Attachments
Patch (3.53 KB, patch)
2012-09-11 08:37 PDT, Philippe Liard
no flags Details | Formatted Diff | Diff
Patch (3.43 KB, patch)
2012-09-11 08:50 PDT, Philippe Liard
no flags Details | Formatted Diff | Diff
Patch (4.52 KB, patch)
2012-09-11 08:58 PDT, Philippe Liard
abarth: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Liard 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.
Comment 1 Peter Beverloo 2012-09-11 08:30:06 PDT
+ the gardeners.
Comment 2 Philippe Liard 2012-09-11 08:37:18 PDT
Created attachment 163373 [details]
Patch
Comment 3 Peter Beverloo 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
Comment 4 Philippe Liard 2012-09-11 08:50:12 PDT
Created attachment 163377 [details]
Patch
Comment 5 Philippe Liard 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.
Comment 6 Peter Beverloo 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.
Comment 7 Philippe Liard 2012-09-11 08:58:22 PDT
Created attachment 163378 [details]
Patch
Comment 8 Peter Beverloo 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.
Comment 9 Philippe Liard 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.
Comment 10 WebKit Review Bot 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
Comment 11 Peter Beverloo 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 :-).
Comment 12 Philippe Liard 2012-09-11 09:39:55 PDT
Ah, sorry :) Thanks for the tip and landing it.
Comment 13 Peter Beverloo 2012-09-11 09:40:24 PDT
Committed r128194: <http://trac.webkit.org/changeset/128194>
Comment 14 Roger Fong 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
Comment 15 Roger Fong 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.
Comment 16 Roger Fong 2012-10-03 11:21:04 PDT
Setting bug as unconfirmed
Comment 17 Roger Fong 2012-10-03 11:45:23 PDT
Meh, I'll just make a new bug, that makes more sense
Comment 18 Roger Fong 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
Comment 19 Philippe Liard 2012-10-04 02:13:28 PDT
Sorry guys for the late reply, I was traveling. I will look at the issue.
Comment 20 Roger Fong 2012-10-04 09:50:35 PDT
Thanks :)