New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 790936 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug

Blocked on:
issue 791696



Sign in to add a comment

Move last-try restart behind a flag

Project Member Reported by peconn@chromium.org, Dec 1 2017

Issue description

I got confused when my test device restarted itself after running instrumentation tests for a change I was writing. My change had made the test fail, but I was trying to do TDD :-P

To make it worse, when I reverted my code (and the tests passed) the device stopped restarting after the tests, leading me to think I'd hit some horrendous Android breaking bug, which I could find no sign of in the device logs.

It turns out that the test class had @RetryOnFailure.

It would be good if we could have some test output explaining that the device has been restarted to retry the test.

I was also running the tests with '--num-retries=0', so restarting the device to retry doesn't make sense. Could we disable this behaviour when num-retries=0?

Also, would it be an idea to add a convenience flag to the test runner such as "developer-run" which developers could use to get behaviour that makes sense on a developer machine (instead of a trybot or tester).
 
Cc: yolandyan@chromium.org
Status: Available (was: Untriaged)
Going to try to repro this scenario locally. I'm not sure that I see how the test runner would wind up restarting your device in that scenario -- we have logic that will reboot it immediately before the last try (https://codesearch.chromium.org/chromium/src/build/android/pylib/local/device/local_device_test_run.py?rcl=0a6d1baa62035faa61680f66b8910c54ee56c7fd&l=120), but w/ --num-retries=0, I don't think we should ever hit that logic.
As for a dev run flag -- that's kind of what --fast-local-dev tries to do: https://codesearch.chromium.org/chromium/src/build/android/test_runner.py?rcl=46a41a802079d801a4d7b3f9d8ad2bf0fd941243&l=176
Yeah, I'm not able to reproduce this locally w/ a failing test and --num-retries=0. If you've got any more details on what you saw, potentially including a patch, that'd be helpful.
Cc: bpastene@chromium.org
Summary: Move last-try restart behind a flag (was: Make RetryOnFailure annotation not restart the device when num-retries=0)
Talked to tedchoc@ and mdjones@, who expressed interest in not having the last-try restart enabled for local test execution. That sounds reasonable to me, and I think the best way to do so at the moment would be to put it behind a flag and pass that to bot test executions.
Owner: bpastene@chromium.org
Status: Assigned (was: Available)
It seems that recovery logic I added is doing more harm than good. Whoops. I'll hide it behind a bot-only flag.
Blockedon: 791696
Project Member

Comment 7 by bugdroid1@chromium.org, Dec 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1ddc023aa30dcedf41e5f9dcc62b351e8712a689

commit 1ddc023aa30dcedf41e5f9dcc62b351e8712a689
Author: Benjamin Pastene <bpastene@chromium.org>
Date: Tue Dec 05 19:44:15 2017

android: Add test runner option to recover devices before last attempt.

Will enable on bots by either:
- adding option to testing spec
- adding option to all isolated tests via mb
- adding option to cmdline in recipe

Leaning towards option 3 atm.

R=jbudorick@chromium.org

Bug: 790936
Change-Id: Ieb9e0e8237e691887e383578f1860cd77e03e5cb
Reviewed-on: https://chromium-review.googlesource.com/804914
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Benjamin Pastene <bpastene@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521791}
[modify] https://crrev.com/1ddc023aa30dcedf41e5f9dcc62b351e8712a689/build/android/pylib/local/device/local_device_environment.py
[modify] https://crrev.com/1ddc023aa30dcedf41e5f9dcc62b351e8712a689/build/android/pylib/local/device/local_device_test_run.py
[modify] https://crrev.com/1ddc023aa30dcedf41e5f9dcc62b351e8712a689/build/android/test_runner.py

Project Member

Comment 8 by bugdroid1@chromium.org, Dec 18 2017

Sign in to add a comment