1347659 - Convert Navigation UITests to XCUITests

Status: RESOLVED FIXED

(Firefox for iOS :: Build & Test, enhancement)

Description

[Isabel Rios[:isabel_rios]]

These tests are disabled and currently not running, the idea is to convert them to XCUITests and have them enabled, up and running

Comment 1

[Isabel Rios[:isabel_rios]]

Attachment: Link to Github pull-request: https://github.com/mozilla-mobile/firefox-ios/pull/2532

Comment 2

[Isabel Rios[:isabel_rios]]

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!

Comment 3

[Isabel Rios[:isabel_rios]]

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!

Comment 4

[No-Jun Park [:njpark]]

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.

Comment 5

[James Hugman [:jhugman] [@jhugman]]

I've added my review in the PR.

Comment 6

[Isabel Rios[:isabel_rios]]

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! :)

Comment 7

[James Hugman [:jhugman] [@jhugman]]

I've added some further comments on the PR. Looking good!

Comment 8

[Isabel Rios[:isabel_rios]]

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!

Comment 9

[Isabel Rios[:isabel_rios]]

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

Thanks jhugman for your help!