Android: uses a fifo rather than logcat for the test runner.

Android: uses a fifo rather than logcat for the test runner.

- This uses the same mechanism as webkit drt.
- It fixes the "timestamping" bug (147800), as the fifo
order is guaranteed (as opposed to logcat).
- It also removes AndroidLogPrinter which was injected in the gtest
framework.
- Downside is that we won't get inline crashstack. However, those
were normally useless as they were not symbolized anyways.
We can still get tombstones if we detect a crash.

BUG=147800
TEST=build/android/run_tests.py


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=157541

[nilesh, 2012-09-18 15:17:40]

On 2012/09/18 15:05:34, bulach wrote:

LGTM
minor comments.

[nilesh, 2012-09-18 15:18:02]

http://codereview.chromium.org/10938014/diff/1/testing/android/native_test_la...
File testing/android/native_test_launcher.cc (right):

http://codereview.chromium.org/10938014/diff/1/testing/android/native_test_la...
testing/android/native_test_launcher.cc:7: // START/END/CRASH of the test suite,
FAILURE/SUCCESS of individual tests etc.
I think this needs to be updated.

http://codereview.chromium.org/10938014/diff/1/testing/android/native_test_la...
testing/android/native_test_launcher.cc:57: // This function runs in a
compromised context. It should not allocate memory.
Now that we have a FIFO, we can use EOF in test scripts to detect crashes.
Please add a TODO in my name to try get rid of signal handler.

[Isaac (away), 2012-09-18 18:44:48]

No inline crash stacks is a bit of a bummer.  Because otherwise you have no way
of correlating which symbolized crash goes with which crash.  Is there any way
to solve that?

[bulach, 2012-09-19 08:43:42]

thanks nilesh! all comments addressed..

ilevy: the stack wasn't symbolized in the first place.. I think the solution is
to eventually add the "stack dump" bot step you wrote downstream. the top-level
frame should indicate which test failed.

http://codereview.chromium.org/10938014/diff/1/testing/android/native_test_la...
File testing/android/native_test_launcher.cc (right):

http://codereview.chromium.org/10938014/diff/1/testing/android/native_test_la...
testing/android/native_test_launcher.cc:7: // START/END/CRASH of the test suite,
FAILURE/SUCCESS of individual tests etc.
On 2012/09/18 15:18:02, nileshagrawal1 wrote:
> I think this needs to be updated.

Done.

http://codereview.chromium.org/10938014/diff/1/testing/android/native_test_la...
testing/android/native_test_launcher.cc:57: // This function runs in a
compromised context. It should not allocate memory.
On 2012/09/18 15:18:02, nileshagrawal1 wrote:
> Now that we have a FIFO, we can use EOF in test scripts to detect crashes.
> Please add a TODO in my name to try get rid of signal handler.

Done.

[commit-bot: I haz the power, 2012-09-19 09:55:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bulach@chromium.org/10938014/6001

[Isaac (away), 2012-09-19 10:02:02]

sgtm.  I'm assuming this patch will get applied downstream, too, where we do
have the stack dump step already.

[bulach, 2012-09-19 10:07:24]

yes! because the patch was a bit risky and we don't have enough coverage
upstream, this has already been applied (and cycled successfully) downstream..
:)

[commit-bot: I haz the power, 2012-09-19 13:04:03]

Change committed as 157541

[Philippe, 2012-09-20 17:27:24]

https://chromiumcodereview.appspot.com/10938014/diff/6001/testing/android/nat...
File testing/android/native_test_launcher.cc (right):

https://chromiumcodereview.appspot.com/10938014/diff/6001/testing/android/nat...
testing/android/native_test_launcher.cc:116: if (mkfifo(fifo_path, 0666)) {
This fails on my device since /mnt/sdcard is a vfat filesystem. 
Which filesystem are you using? Could we use a unix domain socket with an
abstract namespace instead?
Or could we use /data/local/tmp instead maybe?

[bulach, 2012-09-20 17:39:58]

thanks phillippe! yes, myself and peter are working on fixing this... :) sorry
about the breakage!

[Philippe, 2012-09-20 17:45:53]

On 2012/09/20 17:39:58, bulach wrote:
> thanks phillippe! yes, myself and peter are working on fixing this... :) sorry
> about the breakage!

Great, thanks guys! Meanwhile I guess I can just revert the commit locally so
that I can run my unit tests.

[nilesh, 2012-09-20 17:48:16]

On 2012/09/20 17:27:24, Philippe wrote:
>
https://chromiumcodereview.appspot.com/10938014/diff/6001/testing/android/nat...
> File testing/android/native_test_launcher.cc (right):
> 
>
https://chromiumcodereview.appspot.com/10938014/diff/6001/testing/android/nat...
> testing/android/native_test_launcher.cc:116: if (mkfifo(fifo_path, 0666)) {
> This fails on my device since /mnt/sdcard is a vfat filesystem. 
> Which filesystem are you using? Could we use a unix domain socket with an
> abstract namespace instead?
> Or could we use /data/local/tmp instead maybe?

/data/local/tmp is not writable (by appks) on non-rooted devices.

[Philippe, 2012-09-20 17:51:23]

On 2012/09/20 17:48:16, nileshagrawal1 wrote:
> On 2012/09/20 17:27:24, Philippe wrote:
> >
>
https://chromiumcodereview.appspot.com/10938014/diff/6001/testing/android/nat...
> > File testing/android/native_test_launcher.cc (right):
> > 
> >
>
https://chromiumcodereview.appspot.com/10938014/diff/6001/testing/android/nat...
> > testing/android/native_test_launcher.cc:116: if (mkfifo(fifo_path, 0666)) {
> > This fails on my device since /mnt/sdcard is a vfat filesystem. 
> > Which filesystem are you using? Could we use a unix domain socket with an
> > abstract namespace instead?
> > Or could we use /data/local/tmp instead maybe?
> 
> /data/local/tmp is not writable (by appks) on non-rooted devices.

Thanks Nilesh. Good to know.

[bulach, 2012-09-21 12:25:35]

hey guys, I just landed this:
http://codereview.chromium.org/10957036/

all bots seem green, and I tested on a similar device that phillippe and it
passed :)

I hope everything is green now, but please let me know otherwise!