Closed
Bug 1068489
Opened 10 years ago
Closed 10 years ago
Robocop: Provide error message for test failure where device screen may simply be sleeping
Categories
(Firefox for Android Graveyard :: Testing, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 35
People
(Reporter: capella, Assigned: capella)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
nalexander
:
review+
|
Details | Diff | Splinter Review |
A common issue I've noticed seems to be when triggering a robocop test on a device who's screen has gone into power-save / sleep mode. The standard messages provided in that case are as (below), and nothing jumps out hinting @ that possibility. I wonder if we can identify this situation during testing, and generate something like:
" The screen on your device may be sleeping to save power ..."
" If so, turn on the device display and re-start this test"
This might be a "core" bug, but I'll start it here.
-----------------------------------------
Device info: {}
Test root: /storage/sdcard0/tests
Android sdk version '19'; will use this to filter manifests
SUITE-START | Running 1 tests
Error: Could not find debugger None.
pk12util: PKCS12 IMPORT SUCCESSFUL
MochitestServer : launching [u'/home/master/mozilla-central-objff/dist/bin/xpcshell', '-g', '/home/master/mozilla-central-objff/dist/bin', '-v', '170', '-f', '/home/master/mozilla-central-objff/dist/bin/components/httpd.js', '-e', "const _PROFILE_PATH = '/tmp/tmpGyDnjy.mozrunner'; const _SERVER_PORT = '8888'; const _SERVER_ADDR = '192.168.2.5'; const _TEST_PREFIX = undefined; const _DISPLAY_RESULTS = false;", '-f', '/home/master/mozilla-central-droid/_tests/testing/mochitest/server.js']
runtests.py | Server pid: 313
runtests.py | Websocket server pid: 315
runtests.py | SSL tunnel pid: 317
runtests.py | Running tests: start.
Robocop process name: org.mozilla.fennec
INFO | automation.py | Application pid: 7522
SimpleTest START
TEST-START | testSelectionHandler
TEST-PASS | testSelectionHandler | Given message occurred for registered event: {"type":"Gecko:Ready"} - Gecko:Ready should equal Gecko:Ready
EventExpecter: no longer listening for Gecko:Ready
TEST-PASS | testSelectionHandler | url is not null - chrome://roboextender/content/testSelectionHandler.html should not equal null
TEST-PASS | testSelectionHandler | url is not null - chrome://roboextender/content/testSelectionHandler.html should not equal null
TEST-PASS | testSelectionHandler | The toolbar is not in the editing state -
TEST-UNEXPECTED-FAIL | testSelectionHandler | Waiting for Toolbar to enter editing mode. -
TEST-OK | testSelectionHandler | took 8931ms
TEST-START | Shutdown
Passed: 4
Failed: 1
Todo: 0
SimpleTest FINISHED
INFO | automation.py | Application ran for: 0:00:12.230976
INFO | zombiecheck | Reading PID log: /tmp/tmpLxhkNOpidlog
Contents of /data/anr/traces.txt:
MOZ_UPLOAD_DIR not defined; tombstone check skipped
Stopping web server
Stopping web socket server
Stopping ssltunnel
WARNING | leakcheck | refcount logging is off, so leaks can't be detected!
runtests.py | Running tests: end.
Comment 1•10 years ago
|
||
I've seen this, and have a partial fix that works on some of my devices -- see Bug 929654, specifically the patch labeled:
Bug 929654 - Part 3: Turn off keyguard and turn on screen when waiting for Gecko:Ready. r=jmaher
I'd be pretty happy to just fail the test (in setup) if we can detect that the screen is off. I have a patch on that ticket to die if we can't reach the HTTP server.
The reason we need the screen is that we don't start Gecko completely until we get onAttachedToWindow, which doesn't happen if the screen is not on.
Assignee | ||
Comment 2•10 years ago
|
||
Something really simple like this? Not knowing the entire test suite architecture I'd assume checking in UITest and probably PixelTest would be a safe bet.
But is there a blanket rule we can use to determine which tests get protected this way? Not all tests that inherit from BaseTest for example ContentProviderTest/JavascriptTest(s) seem to need this (for ex: testSharedPreferences runs just fine with the screen off -or- on).
Attachment #8492694 -
Flags: feedback?(nalexander)
Comment 3•10 years ago
|
||
Comment on attachment 8492694 [details] [diff] [review]
bug1068489.diff
Review of attachment 8492694 [details] [diff] [review]:
-----------------------------------------------------------------
I would do this for every test, needed or not. Let's make one of the requirements of Robocop be that the screen is on.
::: mobile/android/base/tests/UITest.java
@@ +77,5 @@
> // Helpers depend on components so initialize them first.
> initComponents();
> initHelpers();
> +
> + PowerManager pm = (PowerManager) activity.getSystemService(Context.POWER_SERVICE);
nit: final.
And break out a little helper: throwIfScreenNotOn().
@@ +79,5 @@
> initHelpers();
> +
> + PowerManager pm = (PowerManager) activity.getSystemService(Context.POWER_SERVICE);
> + if (!pm.isScreenOn()) {
> + dumpLog(LOGTAG, "The display on your test device doesn\'t appear to be active,");
Is the \' necessary? I think not.
Also, why not just mAssert.ok(pm.isScreenOn(), "Helpful message that includes the word Robocop.")?
Attachment #8492694 -
Flags: feedback?(nalexander) → feedback+
Assignee | ||
Comment 4•10 years ago
|
||
This seems to do the trick :-) I went ahead and ran it through try ...
https://tbpl.mozilla.org/?tree=Try&rev=d20e77974fcf
Assignee: nobody → markcapella
Attachment #8492694 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8497321 -
Flags: review?(nalexander)
Comment 5•10 years ago
|
||
Comment on attachment 8497321 [details] [diff] [review]
bug1068489.diff
Review of attachment 8497321 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks for the try build. You'd be doing a public service if you updated the patch that tried to connect to the (IP address) of mochi.test and early aborted if it failed. That would help most local immediate failures.
::: mobile/android/base/tests/BaseRobocopTest.java
@@ +114,5 @@
> + * Ensure that the screen on the test device is powered on during tests.
> + */
> + public void throwIfScreenNotOn() {
> + final PowerManager pm = (PowerManager) getActivity().getSystemService(Context.POWER_SERVICE);
> + mAsserter.ok(pm.isScreenOn(), "The display on your test device needs to be powered on.",
Include the magic words "Robocop" in the message, please. (Can't remember the log tag.)
@@ +115,5 @@
> + */
> + public void throwIfScreenNotOn() {
> + final PowerManager pm = (PowerManager) getActivity().getSystemService(Context.POWER_SERVICE);
> + mAsserter.ok(pm.isScreenOn(), "The display on your test device needs to be powered on.",
> + "\nIf it's in sleep mode, turn it on, then Re-start this test.\n");
I'd just go short and sweet: "Robocop tests need the test device screen to be powered on."
Attachment #8497321 -
Flags: review?(nalexander) → review+
Comment 6•10 years ago
|
||
Flagging jmaher (gbrown is on PTO, IIRC) just in case this is a bad idea on infra.
Flags: needinfo?(jmaher)
Assignee | ||
Comment 8•10 years ago
|
||
Comment 9•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•