Add MTE-1520 [v119] Add simple test for Fakespot #16405

clarmso · 2023-09-15T05:14:01Z

📜 Tickets

Jira ticket

💡 Description

Fakespot has been enabled on Amazon.com. Let me write a new test to ensure that Fakespot is present.

📝 Checklist

You have to check all boxes before merging

Filled in the above information (tickets numbers and description of your work)
Updated the PR name to follow our PR naming guidelines
Wrote unit tests and/or ensured the tests suite is passing
When working on UI, I checked and implemented accessibility (minimum Dynamic Text and VoiceOver)
If needed I updated documentation / comments for complex code and public methods

mobiletest-ci-bot · 2023-09-15T05:24:17Z

	Messages
📖	Edited 9 files
📖	Created 1 files

Generated by 🚫 Danger Swift against 9c50036

dragosb01 · 2023-09-15T05:26:05Z

Tests/XCUITests/FakespotTests.swift

+import XCTest
+
+class FakespotTests: BaseTestCase {
+    func testFakespotAvailable() {


Please add the testrail link before the test

If not created yet, we should create the tests there, I already talked to @abodea about this and he agrees on having the test suite there with this basic test for now

I have created the test on TestRail.

thatswinnie · 2023-09-15T06:25:24Z

Tests/XCUITests/FakespotTests.swift

+        // Tap shopping cart icon on Awesome bar
+        // Note: I can't find the label for the shopping cart icon.
+        // Workaround: Tap somewhere left of the "Share" button
+        waitForExistence(app.buttons["TabLocationView.shareButton"])


Could we use the accessibility ids to look for view elements? The id for the button is AccessibilityIdentifiers.Toolbar.shoppingButton.

The app.debugDescription does not show AccessibilityIdentifiers.Toolbar.shoppingButton in the awesome bar. Such an icon is displayed and can be tapped. Here is the debugDescription:

Other, 0x7ff510a1bb60, {{7.0, 53.0}, {376.0, 42.0}} TextField, 0x7ff510a3e0d0, {{48.0, 63.0}, {215.0, 22.0}}, identifier: 'url', label: 'Address Bar', placeholderValue: 'Search or enter address', value: www.amazon.com/gp/... Button, 0x7ff510a3a870, {{343.0, 54.0}, {40.0, 40.0}}, identifier: 'TabLocationView.reloadButton', label: 'Reload page' Button, 0x7ff510a3ac20, {{7.0, 54.0}, {40.0, 40.0}}, identifier: 'TabLocationView.trackingProtectionButton', label: 'Secure connection' Button, 0x7ff510a3afd0, {{303.0, 54.0}, {40.0, 40.0}}, identifier: 'TabLocationView.shareButton', label: 'Share this page'

@thatswinnie has that icon changed the ID, wondering why @clarmso cannot see it when printing the elements... I will give this a try later today

I can't see it either... only Tracking Protection, Share and Reload button are available via print(app.debugDescription)

Relying on the coordinates to tap on the shoppingButton (via tapAtPoint) is flaky on my end. I would not merge this PR before debugDescription could see the icon. 😢

Agreed... that coordinates are specific to a device's screen size.. better to have the ID...

thatswinnie · 2023-09-15T06:27:08Z

Tests/XCUITests/FakespotTests.swift

+        waitForExistence(app.buttons["TabLocationView.shareButton"])
+        app.buttons["TabLocationView.shareButton"].tapAtPoint(CGPoint(x: -10, y: 0))
+        waitForExistence(app.otherElements["PopoverDismissRegion"])
+        waitForExistence(app.staticTexts["Shopping.Sheet.HeaderTitle"])


The title has the a11y id AccessibilityIdentifiers.Shopping.sheetHeaderTitle.

isabelrios · 2023-09-15T09:23:07Z

I run the test locally and it works consistently. Good job here!
I agree that the best place to start with is running it on the Full Functional, we can move it to block PRs on the smoketest when we feel confident about it and the feature not changing.

clarmso · 2023-09-18T13:36:22Z

Tests/XCUITests/FakespotTests.swift

+        // Workaround: Tap somewhere left of the "Share" button
+        waitForExistence(app.buttons[AccessibilityIdentifiers.Toolbar.shareButton])
+        waitUntilPageLoad()
+        app.buttons[AccessibilityIdentifiers.Toolbar.shareButton].tapAtPoint(CGPoint(x: -10, y: 0))


Using the coordinates to tap the shopping icon is flaky.

clarmso · 2023-09-26T20:20:00Z

The test fails intermittently because Amazon may issue a Captcha-like challenge when the site is hit repeatedly. If we don't run the test repeatedly, the intermittent failures are avoidable.

thatswinnie · 2023-09-27T07:41:25Z

Tests/XCUITests/FakespotTests.swift

+
+            // Tap the shopping cart icon
+            waitUntilPageLoad()
+            mozWaitForElementToExist(app.buttons[AccessibilityIdentifiers.Toolbar.shoppingButton])


Tests/XCUITests/FakespotTests.swift

isabelrios · 2023-09-27T09:40:50Z

bitrise.yml

@@ -102,6 +102,7 @@ workflows:

            echo "bitrise_xcode"
            COUNT_XCUI=$(./test-fixtures/generate-metrics.sh /Users/vagrant/git/xcodebuild_fennec.log all)
+            cat /Users/vagrant/git/xcodebuild_fennec.log


Just curious why do we need this?

I was trying to debug the lint warning/error.

isabelrios · 2023-09-27T10:16:05Z

This is looking great @clarmso! Once the IDs are added as @thatswinnie commented, this could be ready to land.
I would not worry for the # of test executions for now because this will run as part of the Full Functional. That said, I think we need to think of a way, if possible, to avoid that issue, let's explore which options we would have.
Thanks both for your work with this PR! 🙏

isabelrios · 2023-09-27T13:14:40Z

Bitrise is red due to the increase in warnings. There is a new warning added in this PR:

/Users/vagrant/git/Tests/XCUITests/FakespotTests.swift:5:8: implicit import of bridging header 'Shared-Bridging-Header.h' via module 'Shared' is deprecated and will be removed in a later version of Swift
import Shared

clarmso · 2023-09-27T13:35:39Z

This is looking great @clarmso! Once the IDs are added as @thatswinnie commented, this could be ready to land. I would not worry for the # of test executions for now because this will run as part of the Full Functional. That said, I think we need to think of a way, if possible, to avoid that issue, let's explore which options we would have. Thanks both for your work with this PR! 🙏

@isabelrios The test fails about 5-6 times out of 100 consecutive test runs. If we only run the test once every few minutes, we may be ok. 🤔

clarmso · 2023-09-27T14:01:02Z

Tests/XCUITests/FakespotTests.swift

+            mozWaitForElementToExist(app.buttons[AccessibilityIdentifiers.Toolbar.shoppingButton])
+            app.buttons[AccessibilityIdentifiers.Toolbar.shoppingButton].tap()
+            mozWaitForElementToExist(app.staticTexts[AccessibilityIdentifiers.Shopping.sheetHeaderTitle])
+            XCTAssertEqual(app.staticTexts[AccessibilityIdentifiers.Shopping.sheetHeaderTitle].label, String.Shopping.SheetHeaderTitle)


@thatswinnie String.Shopping.SheetHeaderTitle is not recognized. Please help. 😞

looks like we need import Shared for that to be recognized but then there is the new warning...

@clarmso if you add the dependency here, it may not warn you ... we did something similar with Common and there is not a warning for that (not sure if this is why though)

clarmso · 2023-09-27T16:04:27Z

Tests/XCUITests/FakespotTests.swift

+            app.buttons[AccessibilityIdentifiers.Toolbar.shoppingButton].tap()
+            mozWaitForElementToExist(app.staticTexts[AccessibilityIdentifiers.Shopping.sheetHeaderTitle])
+            XCTAssertEqual(app.staticTexts[AccessibilityIdentifiers.Shopping.sheetHeaderTitle].label, .Shopping.SheetHeaderTitle)
+


I followed the same style as in the source code here.

firefox-ios/Client/Frontend/Fakespot/FakespotViewController.swift

Line 47 in e44e65b

label.text = .Shopping.SheetHeaderTitle

clarmso · 2023-09-27T16:06:37Z

Tests/XCUITests/FakespotTests.swift

+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at http://mozilla.org/MPL/2.0/
+import XCTest
+import Shared


import Shared is giving me a new warning but the identifier does not work without this import. 🤔

⚠️ /Users/vagrant/git/Tests/XCUITests/FakespotTests.swift:5:8: implicit import of bridging header 'Shared-Bridging-Header.h' via module 'Shared' is deprecated and will be removed in a later version of Swift import Shared

@clarmso we should try to fix this warning before merging, otherwise we will be close to threshold again... I can try to help you with that

…t plans

Co-authored-by: Winnie Teichmann <[email protected]>

isabelrios · 2023-10-02T13:40:23Z

@clarmso please rebase latest main, unit tests failures should be gone with that and we could land this PR

clarmso requested review from isabelrios, jjSDET and dragosb01 September 15, 2023 05:14

clarmso requested review from a team as code owners September 15, 2023 05:14

clarmso mentioned this pull request Sep 15, 2023

Add MTE-1520 [v119] Add simple test for Fakespot #16403

Closed

5 tasks

dragosb01 reviewed Sep 15, 2023

View reviewed changes

thatswinnie reviewed Sep 15, 2023

View reviewed changes

clarmso commented Sep 18, 2023

View reviewed changes

clarmso requested review from dragosb01 and thatswinnie September 26, 2023 20:20

thatswinnie reviewed Sep 27, 2023

View reviewed changes

isabelrios reviewed Sep 27, 2023

View reviewed changes

thatswinnie approved these changes Sep 27, 2023

View reviewed changes

clarmso commented Sep 27, 2023

View reviewed changes

clarmso requested review from thatswinnie and isabelrios September 27, 2023 16:57

clarmso added 5 commits October 2, 2023 10:46

Add FakespotTests

4386004

Disable FakespotTests for ExperimentalIntegration and Performance tes…

5bccaa5

…t plans

Use accessibility identifiers. Add testrail link.

8f57f31

Skip test for iPad

720f066

Update the test to use the icon

7e63fd0

clarmso and others added 8 commits October 2, 2023 10:46

Debug warnings

02c0463

Remove debug

48a4799

Update Tests/XCUITests/FakespotTests.swift

7f90fa5

Co-authored-by: Winnie Teichmann <[email protected]>

Import shared for strings

90ccfc4

Update Tests/XCUITests/FakespotTests.swift

d01f30a

Co-authored-by: Winnie Teichmann <[email protected]>

Remove import Shared statement for lint

75190f5

Fix identifier

f35c310

remove warning for Shared

e63862d

clarmso added 2 commits October 2, 2023 14:05

Merge remote-tracking branch 'upstream/main' into MTE-1520-Fakespot

42b7301

Merge remote-tracking branch 'upstream/main' into MTE-1520-Fakespot

9c50036

isabelrios approved these changes Oct 3, 2023

View reviewed changes

isabelrios merged commit e260331 into mozilla-mobile:main Oct 3, 2023
9 of 10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add MTE-1520 [v119] Add simple test for Fakespot #16405

Add MTE-1520 [v119] Add simple test for Fakespot #16405

clarmso commented Sep 15, 2023

mobiletest-ci-bot commented Sep 15, 2023 •

edited

Loading

dragosb01 Sep 15, 2023

isabelrios Sep 15, 2023

clarmso Sep 18, 2023

thatswinnie Sep 15, 2023

clarmso Sep 15, 2023

isabelrios Sep 18, 2023 •

edited

Loading

isabelrios Sep 18, 2023

clarmso Sep 18, 2023 •

edited

Loading

isabelrios Sep 19, 2023

thatswinnie Sep 15, 2023

isabelrios commented Sep 15, 2023

clarmso Sep 18, 2023

clarmso commented Sep 26, 2023

thatswinnie Sep 27, 2023

isabelrios Sep 27, 2023

clarmso Sep 27, 2023

isabelrios commented Sep 27, 2023

isabelrios commented Sep 27, 2023

clarmso commented Sep 27, 2023

clarmso Sep 27, 2023

isabelrios Sep 27, 2023

isabelrios Sep 27, 2023

clarmso Sep 27, 2023

clarmso Sep 27, 2023

isabelrios Sep 27, 2023 •

edited

Loading

isabelrios commented Oct 2, 2023

Add MTE-1520 [v119] Add simple test for Fakespot #16405

Add MTE-1520 [v119] Add simple test for Fakespot #16405

Conversation

clarmso commented Sep 15, 2023

📜 Tickets

💡 Description

📝 Checklist

mobiletest-ci-bot commented Sep 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

isabelrios Sep 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clarmso Sep 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

isabelrios commented Sep 15, 2023

Choose a reason for hiding this comment

clarmso commented Sep 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

isabelrios commented Sep 27, 2023

isabelrios commented Sep 27, 2023

clarmso commented Sep 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

isabelrios Sep 27, 2023 • edited Loading

Choose a reason for hiding this comment

isabelrios commented Oct 2, 2023

mobiletest-ci-bot commented Sep 15, 2023 •

edited

Loading

isabelrios Sep 18, 2023 •

edited

Loading

clarmso Sep 18, 2023 •

edited

Loading

isabelrios Sep 27, 2023 •

edited

Loading