Closed Bug 887325 Opened 8 years ago Closed 8 years ago

Robocop: Add missing system pages to testSystemPages

Categories

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

ARM
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 25

People

(Reporter: liuche, Assigned: liuche)

Details

Attachments

(1 file, 2 obsolete files)

No description provided.
OS: Mac OS X → Android
Hardware: x86 → ARM
Summary: Add about:healthreport to testSystemPages → Robocop: Add about:healthreport to testSystemPages
Assignee: nobody → liuche
Summary: Robocop: Add about:healthreport to testSystemPages → Robocop: Add missing system pages to testSystemPages
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.
Attachment #771128 - Flags: review?(gbrown)
Attachment #771128 - Attachment is obsolete: true
Attachment #771128 - Flags: review?(gbrown)
Attachment #771129 - Flags: review?(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.
Attachment #771129 - Flags: review?(gbrown) → review-
> 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.
Attachment #771129 - Attachment is obsolete: true
Attachment #772214 - Flags: review?(gbrown)
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 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.
Attachment #772214 - Flags: review?(gbrown) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/eb2c4bc8d051
Status: NEW → ASSIGNED
Target Milestone: --- → Firefox 25
https://hg.mozilla.org/mozilla-central/rev/eb2c4bc8d051
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.