Closed Bug 1042252 Opened 6 years ago Closed 3 years ago

Add some tests for NativeWindow APIs

Categories

(Firefox for Android :: Testing, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED WONTFIX
Firefox 34

People

(Reporter: mfinkle, Assigned: mfinkle)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch nativewin-tests v0.1 (obsolete) — Splinter Review
This was not fun in any way whatsoever, but I have something that tests some code.

Tests:
* toast show
* menu add
* menu callback firing
* menu updating label
* menu remove

https://tbpl.mozilla.org/?tree=Try&rev=2bd13c689bfc
Attachment #8460414 - Flags: review?(wjohnston)
Added a doorhanger test
* Show doorhanger
* With a checkbox
* Uncheck the checkbox
* Watch for the right callback
Assignee: nobody → mark.finkle
Attachment #8460414 - Attachment is obsolete: true
Attachment #8460414 - Flags: review?(wjohnston)
Attachment #8460531 - Flags: review?(wjohnston)
Comment on attachment 8460531 [details] [diff] [review]
nativewin-tests v0.2

Review of attachment 8460531 [details] [diff] [review]:
-----------------------------------------------------------------

Only skimmed over this, but if Finkle spent several hours trying to get this to work in UITest and couldn't, it's probably not worth doing (though the associated improvements to UITest would be nice). I haven't seen many intermittents related to `BaseTest.pressMenuItem` so I imagine this test will be fine. Worst comes to worst, we back it out and port it to UITest anyway. :P

::: mobile/android/base/tests/testNativeWindow.java
@@ +21,5 @@
> +        blockForGeckoReady();
> +
> +        // NOTE: These test methods depend on being run in this order.
> +        addToastTest();
> +        prepNextTest();

nit: I think this would be a little more readable, and easier to keep consistent with a newline between each block:

test();
prepNextTest();

nextTest();
prepNextTest();

@@ +45,5 @@
> +     */
> +    private void addToastTest() {
> +        Actions.EventExpecter eventExpecter = mActions.expectGeckoEvent("TestNativeWindow:ShowToast");
> +        loadUrl(TEST_URL + "#showToast");
> +        eventExpecter.blockForEvent();

The eventExpecters should be unregistered, but I suppose this matters less for tests.

@@ +91,5 @@
> +
> +        // Do a simple search for the menu text on the main menu
> +        mActions.sendSpecialKey(Actions.SpecialKey.MENU);
> +        waitForText("^New Tab$");
> +        mAsserter.ok(mSolo.searchText("^" + MENU_TEXT2 + "$") == false, "Checking for menu", "Menu has been removed");

This could also fail if MENU_TEXT2 just happens to be offscreen. It might be better to do something like AppMenuComponent.findAppMenuItemView [1].

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/tests/components/AppMenuComponent.java?rev=567c1b6a0988#70
Attachment #8460531 - Flags: feedback+
Comment on attachment 8460531 [details] [diff] [review]
nativewin-tests v0.2

Review of attachment 8460531 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry. Forgot this was on my queue. If these pass on try and mcomella is happy, so am I. I'm worried about the menu tests on 2.3 devices though.

::: mobile/android/base/tests/testNativeWindow.java
@@ +28,5 @@
> +        updateMenuTest();
> +        prepNextTest();
> +        removeMenuTest();
> +        prepNextTest();
> +        addDoorhangerTest();

I would personally just move the prepNextTest stuff into each test itself. That said, I'd probably get rid of it and just use Event callbacks to tell the test it was ok to run the "next" item.
Attachment #8460531 - Flags: review?(wjohnston) → review+
(In reply to Wesley Johnston (:wesj) from comment #3)

> Sorry. Forgot this was on my queue. If these pass on try and mcomella is
> happy, so am I. I'm worried about the menu tests on 2.3 devices though.

BaseTest.selectMenuItem has code to handle the 2.3 "More" menu situation

> I would personally just move the prepNextTest stuff into each test itself.
> That said, I'd probably get rid of it and just use Event callbacks to tell
> the test it was ok to run the "next" item.

I tried removing it, but found that only the first test case was ever run. I found that I MUST load a different page (I picked about:blank) between the test cases. I already do use events/messages to tell the Java code to run the next test.

I wish writing tests was easy as we hope they'd be to write.
https://hg.mozilla.org/mozilla-central/rev/b17854552298
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
Reverted for intermittent test failures (this failure was present on the try run too...):

Android 2.3 Emulator mozilla-inbound opt test robocop-3 on 2014-08-20 18:48:58 PDT for push 5b6dc6fbc556

slave: tst-linux64-spot-888

https://tbpl.mozilla.org/php/getParsedLog.php?id=46421181&tree=Mozilla-Inbound

{
19:22:37     INFO -  785 INFO SimpleTest START
19:22:37     INFO -  786 INFO TEST-START | testNativeWindow
19:22:37     INFO -  787 INFO TEST-PASS | testNativeWindow | Given message occurred for registered event: {"type":"Gecko:Ready"} - Gecko:Ready should equal Gecko:Ready
19:22:37     INFO -  788 INFO EventExpecter: no longer listening for Gecko:Ready
19:22:37     INFO -  789 INFO TEST-PASS | testNativeWindow | Given message occurred for registered event: {"type":"TestNativeWindow:ShowToast"} - TestNativeWindow:ShowToast should equal TestNativeWindow:ShowToast
19:22:37     INFO -  790 INFO waitForText timeout on TOAST!
19:22:37     INFO -  791 INFO TEST-UNEXPECTED-FAIL | testNativeWindow | Waiting for the toast - Toast has been displayed
19:22:37     INFO -  TEST-INFO | expected PASS
19:22:37     INFO -  792 ERROR Exception caught during test! - junit.framework.AssertionFailedError: TEST-UNEXPECTED-FAIL | testNativeWindow | Waiting for the toast - Toast has been displayed
19:22:37     INFO -  793 INFO TEST-UNEXPECTED-FAIL | testNativeWindow | Exception caught - junit.framework.AssertionFailedError: TEST-UNEXPECTED-FAIL | testNativeWindow | Waiting for the toast - Toast has been displayed
19:22:37     INFO -  TEST-INFO | expected PASS
19:22:37     INFO -  794 INFO TEST-OK | testNativeWindow | took 65394ms
19:22:37     INFO -  795 INFO TEST-START | Shutdown
19:22:37     INFO -  796 INFO Passed: 2
19:22:37  WARNING -  797 INFO Failed: 2
19:22:37  WARNING -  One or more unittests failed.
19:22:37     INFO -  798 INFO Todo: 0
19:22:37     INFO -  799 INFO SimpleTest FINISHED
19:22:37     INFO -  INFO | automation.py | Application ran for: 0:01:16.988732
}

Logcat:
http://mozilla-releng-blobs.s3.amazonaws.com/blobs/mozilla-inbound/sha512/edc282fbc1ea911b900d740a4788d2e5991ba268103dd51b5214e35be711a9ab4781b30b546c5c5ef1d2db305e7fb72e2e84f60aeb8c4086619e596f26c5cb07

remote:   https://hg.mozilla.org/integration/fx-team/rev/f734fbcc3de7
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Duplicate of this bug: 1055886
I don't have much insight into the issues here, but I have had trouble with checkboxes in Robocop in the past. testDoorHanger was plagued by intermittents until we did this -- http://hg.mozilla.org/mozilla-central/file/c14e5feadc61/mobile/android/base/tests/testDoorHanger.java#l163
With the advent of Web Extensions, we're definitely not going to land this.
Status: REOPENED → RESOLVED
Closed: 6 years ago3 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.