Implement Robocop tests for the tab strip

NEW
Unassigned

Status

()

Firefox for Android
General
4 years ago
3 years ago

People

(Reporter: lucasr, Unassigned)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Comment hidden (empty)
(Reporter)

Updated

4 years ago
No longer depends on: 1014987
(Reporter)

Updated

3 years ago
Assignee: lucasr.at.mozilla → nobody
Do this last for new-tablet-v1: uplifting tests is not as dirty as uplifting user-facing changes.
Created attachment 8557218 [details] [diff] [review]
Bug 1064828 - Implement Robocop tests for the new tab strip

My first robocop test.

Just wanted to get an idea on how it's looking so far and where to go next.  It's really not finished, I know, but wanted to get you to have a look so I can have something to get cracking on on Monday
Attachment #8557218 - Flags: feedback?(michael.l.comella)
Comment on attachment 8557218 [details] [diff] [review]
Bug 1064828 - Implement Robocop tests for the new tab strip

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

Just happened to notice this...

::: mobile/android/base/tests/robocop.ini
@@ +138,2 @@
>  # disabled on Android 2.3; bug 946656
>  skip-if = android_version == "10"

Careful! skip-if follows the test it applies to. You accidentally enabled testAboutHomeVisibility on 2.3 and skipped your test on 2.3!!
Good spot Geoff - thanks!
Comment on attachment 8557218 [details] [diff] [review]
Bug 1064828 - Implement Robocop tests for the new tab strip

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

Looks to be on the right track! :)

::: mobile/android/base/tests/UITest.java
@@ +95,4 @@
>          mAppMenu = new AppMenuComponent(this);
>          mGeckoView = new GeckoViewComponent(this);
>          mToolbar = new ToolbarComponent(this);
> +        mTabStrip = new TabStripComponent(this);

nit: Alphabetize, pretty much everywhere TabStrip is initialized.

::: mobile/android/base/tests/components/AppMenuComponent.java
@@ +39,5 @@
>          FORWARD(R.string.forward),
>          NEW_TAB(R.string.new_tab),
>          PAGE(R.string.page),
> +        RELOAD(R.string.reload),
> +        NEW_PRIVATE_TAB(R.string.new_private_tab);

nit: Preferably order this like the menu - move reload up too, if you don't mind.

::: mobile/android/base/tests/components/TabStripComponent.java
@@ +54,5 @@
> +
> +        return this;
> +    }
> +
> +    public TabStripComponent clickAddTab() {

nit: -> addTab

@@ +77,5 @@
> +        getTabsListener().blockUntilTriggered();
> +        return this;
> +    }
> +
> +    public TabStripComponent closeTab(int index) {

Generally, accessing items by index is risky business because indices are not always guaranteed to be the same. However, this seems like an okay use.

@@ +80,5 @@
> +
> +    public TabStripComponent closeTab(int index) {
> +        assertVisible();
> +        fAssertTrue("The number of tabs is greater than the index of the tab being closed",
> +                getTabStrip().getAdapter().getCount() > index);

Also assert the index >= 0.

@@ +124,5 @@
> +        getTabsListener().blockUntilTriggered();
> +        return this;
> +    }
> +
> +    public void finish() {

This should be hidden in the framework, but I don't think it's necessary - we shouldn't need to hook into the underlying state mechanisms - see below.

@@ +155,5 @@
> +        }
> +        return tabsListener;
> +    }
> +
> +    private class TabsListener implements Tabs.OnTabsChangedListener {

If possible, you want to assert what is visible on the screen, not what events are taking place. For example, just because we receive an event doesn't mean the UI changed to compensate for that event (e.g. we're the first listener to run and second listener triggers a layout call).

e.g. I'd wait for the Private browsing text to appear on the screen, or about:home to appear (if it's not already visible)

::: mobile/android/base/tests/testTabStrip.java
@@ +28,5 @@
> +
> +
> +        mTabStrip.assertNumberOfTabs(3);
> +
> +        mTabStrip.addPrivateTabThroughMenu(mAppMenu);

Don't go through the tab strip to do this here - just do it straight through AppMenuComponent.
Attachment #8557218 - Flags: feedback?(michael.l.comella) → feedback+
Blocks: 1024426
No longer blocks: 1014156
Summary: Implement Robocop tests for the new tab strip → Implement Robocop tests for the tab strip
You need to log in before you can comment on or make changes to this bug.