Closed
Bug 1064828
Opened 10 years ago
Closed 4 years ago
Implement Robocop tests for the tab strip
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
INCOMPLETE
People
(Reporter: lucasr, Unassigned)
References
Details
Attachments
(1 file)
(deleted),
patch
|
mcomella
:
feedback+
|
Details | Diff | Splinter Review |
No description provided.
Reporter | ||
Updated•10 years ago
|
Assignee: lucasr.at.mozilla → nobody
Comment 1•10 years ago
|
||
Do this last for new-tablet-v1: uplifting tests is not as dirty as uplifting user-facing changes.
Comment 2•10 years ago
|
||
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 3•10 years ago
|
||
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!!
Comment 4•10 years ago
|
||
Good spot Geoff - thanks!
Comment 5•10 years ago
|
||
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+
Updated•9 years ago
|
Summary: Implement Robocop tests for the new tab strip → Implement Robocop tests for the tab strip
Comment 6•4 years ago
|
||
We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INCOMPLETE
Assignee | ||
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•