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)

ARM
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 35

People

(Reporter: capella, Assigned: capella)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
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.
Attached patch bug1068489.diff (obsolete) (deleted) — Splinter Review
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 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+
Attached patch bug1068489.diff (deleted) — Splinter Review
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 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+
Flagging jmaher (gbrown is on PTO, IIRC) just in case this is a bad idea on infra.
Flags: needinfo?(jmaher)
I don't see anything wrong with this approach :)
Flags: needinfo?(jmaher)
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: