Robocop: Add test to show Title Bar options

RESOLVED FIXED in Firefox 29

Status

()

Firefox for Android
General
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: TeoVermesan, Assigned: TeoVermesan)

Tracking

unspecified
Firefox 29
ARM
Android
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

4 years ago
This bug will cover an automated test for showing the full URL and title in the URL Bar.
MozTrap testcase: https://moztrap.mozilla.org/manage/case/8076/
(Assignee)

Comment 1

4 years ago
Created attachment 8350566 [details] [diff] [review]
Title Bar Options

The patch tests the Title Bar Options showing the full URL and title in the URL Bar.
I have run the tests locally on multiple devices and there were no failures.
The try-server is all green: https://tbpl.mozilla.org/?tree=Try&rev=ae31597cba4b



Should it be stand alone or integrated with another test?
Attachment #8350566 - Flags: review?(gbrown)
Sorry, I won't have a chance to look at this until after the holidays. At a glance, I think it would be better to integrate this with testSettingsMenuItems.
Comment on attachment 8350566 [details] [diff] [review]
Title Bar Options

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

::: mobile/android/base/tests/testTitleBar.java
@@ +40,5 @@
> +
> +    // Entering settings, changing the options: show title/page address option and verifing the device type because for phone there is an extra back action to exit the settings menu
> +    public void selectOption(String option) {
> +        selectSettingsItem(StringHelper.DISPLAY_SECTION_LABEL, StringHelper.TITLE_BAR_LABEL);
> +        mAsserter.ok(waitForText(StringHelper.TITLE_BAR_LABEL), "Waiting for the pop-up to open", "Pop up with the options was openend");

selectSettingsItem() will click to open the Display settings, which include the settings item "Title bar". It will then select "Title bar" to open the pop-up which is titled "Title bar". I expect the waitForText("Title bar") is waiting for the pop-up to open, but even if it has not opened yet, waitForText will succeed because "Title bar" is on the Display settings page. I think you would wait for something else, like "Show page title" instead.

@@ +42,5 @@
> +    public void selectOption(String option) {
> +        selectSettingsItem(StringHelper.DISPLAY_SECTION_LABEL, StringHelper.TITLE_BAR_LABEL);
> +        mAsserter.ok(waitForText(StringHelper.TITLE_BAR_LABEL), "Waiting for the pop-up to open", "Pop up with the options was openend");
> +        mSolo.clickOnText(option);
> +        mAsserter.ok(waitForText(StringHelper.TITLE_BAR_LABEL), "Waiting to press the option", "The pop-up is dismissed once clicked");

Here there is the reverse problem: waitForText("Title bar") will succeed if the pop-up is open or closed.

@@ +45,5 @@
> +        mSolo.clickOnText(option);
> +        mAsserter.ok(waitForText(StringHelper.TITLE_BAR_LABEL), "Waiting to press the option", "The pop-up is dismissed once clicked");
> +        if (mDevice.type.equals("phone")) {
> +            mActions.sendSpecialKey(Actions.SpecialKey.BACK);
> +            mAsserter.ok(waitForText(StringHelper.DISPLAY_SECTION_LABEL), "Waiting to perform one back", "One back performed");

Again..."Display" is found both on the main Settings page and as the title of the Display settings -- you should not use that to distinguish between being on one page or the other.
Attachment #8350566 - Flags: review?(gbrown) → review-
(Assignee)

Comment 4

4 years ago
Created attachment 8360413 [details] [diff] [review]
Title Bar Options

I made the  requested changes and the try server is green: https://tbpl.mozilla.org/?tree=Try&rev=07a1c1fd73a9

I think testSettingsMenuItems is too large. It only verifies that the sections in the menu are present and it doesn't tests its functionality. Is this test too short to be independent? Or should be integrated with another test?
Attachment #8350566 - Attachment is obsolete: true
Attachment #8360413 - Flags: review?(gbrown)
Comment on attachment 8360413 [details] [diff] [review]
Title Bar Options

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

Thanks -- looks good now. It's fine to keep this as a separate test if you like.

::: mobile/android/base/tests/testTitleBar.java
@@ +1,4 @@
> +package org.mozilla.gecko.tests;
> +import android.view.View;
> +import org.mozilla.gecko.*;
> +import java.util.ArrayList;

nit -- I think you can remove the android.view.View and java.util.ArrayList imports

@@ +6,5 @@
> +/**
> + * This patch tests the option that shows the full URL and title in the URL Bar
> + */
> +
> +public class testTitleBar extends PixelTest {

Does this need to be a PixelTest? It is okay if it does, but I think you could inherit from BaseTest.
Attachment #8360413 - Flags: review?(gbrown) → review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/478c4361aa0b
Assignee: nobody → teodora.vermesan
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/478c4361aa0b
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 29
You need to log in before you can comment on or make changes to this bug.