887325 - Robocop: Add missing system pages to testSystemPages

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Theme and Visual Design, defect)

Comment 1

[Chenxia Liu [:liuche]]

Attachment: Patch: add various system pages and restructure test v1

Added about:healthreport, aboug:feedback, and restructured the test and some stuff in BaseTest for accessing Settings more easily.

Also removed the boolean to check for More/Tools, because some of the system pages are in Tools, and some of them are in Settings, and that will cause breakage when testing Tools pages before Settings pages.

Comment 2

[Chenxia Liu [:liuche]]

Attachment: Patch: add various system pages and restructure test v1

Comment 3

[Chenxia Liu [:liuche]]

Try build: https://tbpl.mozilla.org/?tree=Try&rev=b6fd7b63a60d

Comment 4

[Geoff Brown [:gbrown]]

Comment on attachment 771129 [details] [diff] [review]
Patch: add various system pages and restructure test v1

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

Need to work around the copyOfRange limitation and get a clean try run.

::: mobile/android/base/tests/BaseTest.java.in
@@ +129,5 @@
>          mDriver = new FennecNativeDriver(mActivity, mSolo, rootPath);
>          mActions = new FennecNativeActions(mActivity, mSolo, getInstrumentation(), mAsserter);
>          mDevice = new Device();
> +        midWidth = mDriver.getGeckoWidth()/2;
> +        midHeight = mDriver.getGeckoHeight()/2;

Let's name these mMidWidth, mMidHeight.

@@ +450,5 @@
> +
> +    /**
> +     * Traverses the items in listItems in order in the menu.
> +     */
> +    public void selectItemFromMenu(String[] listItems) {

I might call this "selectMenuItems" instead

@@ +456,5 @@
> +        if (listLength > 0) {
> +            selectMenuItem(listItems[0]);
> +        }
> +        if (listLength > 1) {
> +            for (String item : Arrays.copyOfRange(listItems, 1, listLength)) {

As the try run shows, copyOfRange is only available in Android 9+; we must support 8.

@@ +542,5 @@
> +     * solo.scroll* does not work in dialogs.
> +     */
> +    public void scrollDown() {
> +        MotionEventHelper meh = new MotionEventHelper(getInstrumentation(), mDriver.getGeckoLeft(), mDriver.getGeckoTop());
> +        meh.dragSync(midWidth, midHeight+100, midWidth, midHeight-100);

I do not like this function:
 - the fixed +/- 100 -- might that be too much or too little on some devices?
 - "scrollDown" sounds like a general purpose utility, but the function is actually pretty trivial and designed for a specific purpose

I would prefer to use some robotium facility, but I'm not sure what will work / what to suggest -- just something to consider.

Comment 5

[Chenxia Liu [:liuche]]

Attachment: Patch: add various system pages and restructure test v2

> Need to work around the copyOfRange limitation and get a clean try run.

This patch uses indices instead of copyOfRange.

>  +    public void selectItemFromMenu(String[] listItems) {
> I might call this "selectMenuItems" instead

Updated the method name to selectMenuItemByPath, so as not to conflict with the existing-but-slightly-different method selectMenuItem.

> Let's name these mMidWidth, mMidHeight.

I removed the part of the patch that touches this, but I can add the variable rename if you'd like.

> I would prefer to use some robotium facility, but I'm not sure what will work 
> what to suggest -- just something to consider.

You're absolutely right, I forgot that the hack was for dialogs only, so mSolo.scrollDown works fine for the settings menu.

Comment 6

[Chenxia Liu [:liuche]]

try: https://tbpl.mozilla.org/?tree=Try&rev=64d4db24c985

Comment 7

[Chenxia Liu [:liuche]]

I realized inbound was closed earlier, so the previous try build might be building some broken tests.

try build not based on broken inbound: https://tbpl.mozilla.org/?tree=Try&rev=1d7a5f295a91

Comment 8

[Geoff Brown [:gbrown]]

Comment on attachment 772214 [details] [diff] [review]
Patch: add various system pages and restructure test v2

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

This looks great now.

Comment 9

[Chenxia Liu [:liuche]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/eb2c4bc8d051

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/eb2c4bc8d051