Convert Navigation UITests to XCUITests

RESOLVED FIXED

Status

()

Firefox for iOS
Build & Test
RESOLVED FIXED
7 months ago
6 months ago

People

(Reporter: isabel_rios, Assigned: isabel_rios)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

7 months ago
These tests are disabled and currently not running, the idea is to convert them to XCUITests and have them enabled, up and running
(Assignee)

Comment 1

7 months ago
Created attachment 8848487 [details] [review]
Link to Github pull-request: https://github.com/mozilla-mobile/firefox-ios/pull/2532
(Assignee)

Comment 2

7 months ago
Comment on attachment 8848487 [details] [review]
Link to Github pull-request: https://github.com/mozilla-mobile/firefox-ios/pull/2532

There is question about the issue removing the NavigationTests file, which class is called by Global.swift, seems it is possible to replace it by another class existing in the UI since the tests pass, but confirmation would be needed and also to understand how it works. But in the meantime if you could please do a first review to check if the rest looks fine, that would help! Thanks!
Attachment #8848487 - Attachment description: pull-request → Link to Github pull-request: https://github.com/mozilla-mobile/firefox-ios/pull/2532
Attachment #8848487 - Flags: review?(npark)
(Assignee)

Comment 3

7 months ago
Comment on attachment 8848487 [details] [review]
Link to Github pull-request: https://github.com/mozilla-mobile/firefox-ios/pull/2532

Once the doubt has been answered and the Global.swift can be modified as in the PR, asking also for review to jhugman. A new method has been added to FxScreenGraph and would be good to have your input and opinion about modifying the tests with the new scenes added to FxScreenGraph by you or leave them as they are.
Thanks!
Attachment #8848487 - Flags: review?(jhugman)

Comment 4

7 months ago
Comment on attachment 8848487 [details] [review]
Link to Github pull-request: https://github.com/mozilla-mobile/firefox-ios/pull/2532

Other than one minor comment, squash commit, it looks good to me.
Attachment #8848487 - Flags: review?(npark) → review+
Attachment #8848487 - Flags: review?(jhugman) → feedback+
I've added my review in the PR.

Updated

7 months ago
Blocks: 1351075

Updated

7 months ago
No longer blocks: 1351075
(Assignee)

Comment 6

6 months ago
Hi James, sorry for asking again but have some questions on the PR that would need your help to answer and so modify the PR with the requested changes. I know you are super busy with the push stuff but if you could please have a look whenever you have some time I would really appreciate it. This and the ReaderViewUI test suite are the only left to convert. Thanks! :)
Flags: needinfo?(jhugman)
I've added some further comments on the PR. Looking good!
Flags: needinfo?(jhugman)
(Assignee)

Comment 8

6 months ago
Thanks for the review! I tried to fix them. Some comments added and a question regarding one accessibility ID I did not found and had to define.
Hopefully it would be better now and would not take too much more ouf your time.

Thanks again for your help!
(Assignee)

Comment 9

6 months ago
Merged https://github.com/mozilla-mobile/firefox-ios/commit/6b6f772e17c63ecca190d7a849ef2fcfc0c6237b

Thanks jhugman for your help!
Status: NEW → RESOLVED
Last Resolved: 6 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.