Bug 96471 - [chromium] When running layout tests on android judge about installation success from adb command exit status
Summary: [chromium] When running layout tests on android judge about installation succ...
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-11 22:56 PDT by Egor Pasko
Modified: 2012-09-14 09:50 PDT (History)
5 users (show)

See Also:


Attachments
Patch (3.96 KB, patch)
2012-09-12 00:02 PDT, Egor Pasko
no flags Details | Formatted Diff | Diff
[chromium] When running layout tests on android judge about installation success from adb command exit status (3.69 KB, patch)
2012-09-12 00:29 PDT, Egor Pasko
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Egor Pasko 2012-09-11 22:56:27 PDT
[chromium] When running layout tests on android judge about installation success from adb command exit status
Comment 1 Egor Pasko 2012-09-12 00:02:57 PDT
Created attachment 163528 [details]
Patch
Comment 2 Egor Pasko 2012-09-12 00:29:15 PDT
Created attachment 163535 [details]
[chromium] When running layout tests on android judge about installation success from adb command exit status

previous patch could not apply
Comment 3 Peter Beverloo 2012-09-14 05:49:42 PDT
Hi Egor,

Thank you for the patch! I tried to see if we could rely on the exit codes, and got the result listed at the bottom of this message. It returned "0" regardless of whether the installation succeeded, so I'm afraid it may not be possible to rely on it being right.

A concern I have is that we need to execute the same command twice in order to get the command's output. This does not reproduce on the bots, and as far as I'm aware it neither reproduces for anyone locally: we've been using this code for quite a while now, and various people have ran layout tests.

Could you emphasize on the set up you have, and the output it produces for that environment? I'm interested in seeing whether we can find a better solution, but would like to understand the situation for that.

---

beverloo /buildbot/WebKit  $ adb install -r out/Release/TestWebKitAPI_apk/TestWebKitAPI-debug.apk 
echo6005 KB/s (10398532 bytes in 1.691s)
	pkg: /data/local/tmp/TestWebKitAPI-debug.apk
Success
beverloo /buildbot/WebKit  $ echo $?
0
beverloo /buildbot/WebKit  $ adb install out/Release/TestWebKitAPI_apk/TestWebKitAPI-debug.apk 
5597 KB/s (10398532 bytes in 1.814s)
	pkg: /data/local/tmp/TestWebKitAPI-debug.apk
Failure [INSTALL_FAILED_ALREADY_EXISTS]
beverloo /buildbot/WebKit  $ echo $?
0
Comment 4 Egor Pasko 2012-09-14 06:17:54 PDT
Peter,

it looks like my version of adb is different, but it appears from the dependency pulled into my tree:
$ which adb
/[...]/clank/external/chrome/third_party/android_tools/sdk//platform-tools/adb

And I never get "Success" or "Failure" in adb's stdout:

$ adb install -r out/Debug/DumpRenderTree_apk/DumpRenderTree-debug.apk; echo $?
4287 KB/s (12481666 bytes in 2.843s)
0

$ adb install out/Debug/DumpRenderTree_apk/DumpRenderTree-debug.apk; echo $?
4343 KB/s (12481666 bytes in 2.806s)
0

$ adb install /does/not/exist; echo $?
can't find '/does/not/exist' to install
1

so my version of adb seems to be less verbose and at the same time correctly sets the exit code. It would be nice to find the cause for such difference.
Comment 5 Peter Beverloo 2012-09-14 06:18:50 PDT
I see. Where did you install ADB from? Are you using a Mac by any chance?

beverloo WebKit (fifo-test-runners) $ adb --version
Android Debug Bridge version 1.0.29
Comment 6 Egor Pasko 2012-09-14 06:29:28 PDT
adb --version 2>&1 | head -1
Android Debug Bridge version 1.0.29

As I wrote (see result of `which adb`), the adb is coming from the Android SDK that is in the chrome tree (pulled via git deps).

Could it be that the environment is set up differently? I am out of ideas.

If there are two valid adb modes with said behavior we would become more careful:
1. check if we encountered "Success" or "Failure" (we are with the "old" adb mode)
2. else judge by the exit code (we are with the adb mode that Egor is having)

I don't like how it looks/reads but really I do not see other ways how this can be transitioned from one mode to another smoothly given the assumption that two modes of adb are correct.
Comment 7 Egor Pasko 2012-09-14 06:29:54 PDT
and it's Linux
Comment 8 Peter Beverloo 2012-09-14 06:39:36 PDT
(In reply to comment #4)
> $ adb install -r out/Debug/DumpRenderTree_apk/DumpRenderTree-debug.apk; echo $?
> 4287 KB/s (12481666 bytes in 2.843s)
> 0
> 
> $ adb install out/Debug/DumpRenderTree_apk/DumpRenderTree-debug.apk; echo $?
> 4343 KB/s (12481666 bytes in 2.806s)
> 0

For clarity: the second install will have failed, as the package was already installed. It should have shown "Failure [INSTALL_FAILED_ALREADY_EXISTS]"...
Comment 9 Peter Beverloo 2012-09-14 06:44:20 PDT
(In reply to comment #6)
> adb --version 2>&1 | head -1
> Android Debug Bridge version 1.0.29
> 
> As I wrote (see result of `which adb`), the adb is coming from the Android SDK that is in the chrome tree (pulled via git deps).
> 
> Could it be that the environment is set up differently? I am out of ideas.
> 
> If there are two valid adb modes with said behavior we would become more careful:
> 1. check if we encountered "Success" or "Failure" (we are with the "old" adb mode)
> 2. else judge by the exit code (we are with the adb mode that Egor is having)
> 
> I don't like how it looks/reads but really I do not see other ways how this can be transitioned from one mode to another smoothly given the assumption that two modes of adb are correct.

I have no idea why this is happening, maybe adb is crashing? Could you try to run adb in gdb to see if you get any crash stacks or other output? It seems very unlikely that ADB has a verbose and non-verbose mode, I fairly sure that's the case.

Other test suites in Chromium also check for "Success" in the command's output, so you should be able to rely on that. I suspect communication with the device may be timing out as well, have you tried reconnecting it, or using a different USB cable?
Comment 10 Egor Pasko 2012-09-14 08:19:06 PDT
(In reply to comment #8)
> (In reply to comment #4)
> > $ adb install -r out/Debug/DumpRenderTree_apk/DumpRenderTree-debug.apk; echo $?
> > 4287 KB/s (12481666 bytes in 2.843s)
> > 0
> > 
> > $ adb install out/Debug/DumpRenderTree_apk/DumpRenderTree-debug.apk; echo $?
> > 4343 KB/s (12481666 bytes in 2.806s)
> > 0
> 
> For clarity: the second install will have failed, as the package was already installed. It should have shown "Failure [INSTALL_FAILED_ALREADY_EXISTS]"...

It does not fail for me, good to know that it should though, thanks!

I looked at strace of the adb command and I see that there is no crash. From the adb server we successfully get the string "4702 KB/s (12481666 bytes in 2.592s" along with a reply "OKAY". Prints the former and exits.

I also looked at the adb server strace, also looks that it works without any exceptions doing:
1. transfer the apk via transport
2. shell:pm install /data/local/tmp/...
3. shell:rm /data/local/tmp/DumpRenderTree...apk
4. gets OKAY as result

maybe I should re-flash the device with some certain firmware?
Comment 11 Egor Pasko 2012-09-14 09:50:06 PDT
found a way to fix it on the side of the device