Closed Bug 1183970 Opened 7 years ago Closed 6 years ago

Test for deleting from top sites

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
fxios 1.1+ ---

People

(Reporter: wesj, Assigned: sleroux)

Details

Attachments

(1 file, 2 obsolete files)

We should have a ui test for deleting a tile from top sites. At the very least, it'll tell us if there's a crash :)
Attachment #8633882 - Flags: review?(bnicholson)
Component: Testing → Build & Test
Product: Firefox for Android → Firefox for iOS
Version: Trunk → unspecified
Assignee: nobody → wjohnston
Status: NEW → ASSIGNED
Comment on attachment 8633882 [details] [review]
PR https://github.com/mozilla/firefox-ios/pull/740

Test looks fine to me, but we have some infrastructure issues that could cause us some pain.
Attachment #8633882 - Flags: review?(bnicholson) → feedback+
Flags: needinfo?(etoop)
Assignee: wjohnston2000 → nobody
Status: ASSIGNED → NEW
This works just fine when run as part of the larger suite on both iPhone and iPad so I say :+1:
Flags: needinfo?(etoop)
Assignee: nobody → bmunar
Fixing nits and adding 1.1 milestone to merge after string freeze!
Attached file NEW PR (obsolete) —
Attachment #8649028 - Flags: review?(bnicholson)
Attachment #8633882 - Attachment is obsolete: true
Attachment #8633882 - Flags: review?(bnicholson)
Attachment #8649028 - Flags: review?(bnicholson) → review+
yo yo yo, is a branch ready for dis to merge?
Assignee: brymunar → sleroux
Status: NEW → ASSIGNED
Comment on attachment 8649028 [details]
NEW PR

Made old patch obsolete as new patch fixes test case error
Attachment #8649028 - Attachment is obsolete: true
I pulled down Bryan's latest PR and both test cases were broken. Looks like the test relied on the page title on the top sites cell where now we show the domain. I fixed the delete top site test case by replacing the label it was looking for from "Page 1" -> "127.0.0.1" but I couldn't fix the basic UI test case because navigating to 127.0.0.1 will cause the error page to show since it's not a valid page. I've removed that test case for now and kept the deletion one. Thoughts?
Attachment #8658272 - Flags: review?(bnicholson)
Comment on attachment 8658272 [details] [review]
https://github.com/mozilla/firefox-ios/pull/1044

LGTM -- I hesitated to land this before because I wasn't sure how the NSLocalizedString change here would affect l10n exports. Takeaway from IRC was that this should land on master and the string itself should be uplifted to the v1.0 branch, but please double-check that that's correct.
Attachment #8658272 - Flags: review?(bnicholson) → review+
No point in churning l10n strings for a test; bumping this to 1.1.
Sounds good - ya I was only going to merge this into master (1.1) and not port it back since it's just a test.
Merged
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.