-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor MTE-3854 Integrate menu refactor work and xcode 16 #23381
base: main
Are you sure you want to change the base?
Conversation
Generated by 🚫 Danger Swift against 609d794 |
@@ -119,6 +119,7 @@ | |||
"ShortcutRouteTests\/testOpenLastBookmarkShortcutWithInvalidUrl()", | |||
"TabManagerTests\/testDeleteSelectedTab()", | |||
"TabManagerTests\/testPrivatePreference_togglePBMDeletesPrivate()", | |||
"TabToolbarHelperTests\/testTelemetryForSiteMenu()", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test fails with the menu refactor turned on. 😞
@isabelrios Should we merge this PR just with the changes for smoke tests? 🙂 Please approve this PR if it's the case. We can address the full functional tests in a subsequent PR. |
This pull request has conflicts when rebasing. Could you fix it @clarmso? 🙏 |
b2c6b6a
to
373343b
Compare
…dressPAM in the screen graph)
334372c
to
f8c6e01
Compare
XCUIDevice.shared.orientation = .landscapeLeft | ||
mozWaitForElementToExist(hamburgerMenu) | ||
mozWaitForElementToNotExist(app.tables["Context Menu"]) | ||
// mozWaitForElementToNotExist(app.tables["Context Menu"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we add a comment why we comment these lines?
@@ -58,25 +61,25 @@ class ToolbarMenuTests: BaseTestCase { | |||
"Menu button is not below the pocket cells area" | |||
) | |||
hamburgerMenu.tap() | |||
mozWaitForElementToExist(app.tables["Context Menu"]) | |||
mozWaitForElementToExist(app.images[StandardImageIdentifiers.Large.avatarCircle]) | |||
// mozWaitForElementToExist(app.tables["Context Menu"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
[ | ||
app.staticTexts["mozilla.org"], | ||
app.staticTexts["Secure connection"] | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not change the indentation (unless it's swiftlint related).
📜 Tickets
Jira ticket
💡 Description
Cherry pick menu refactor work after Xcode 16 is in use.
TODO
📝 Checklist
You have to check all boxes before merging
@Mergifyio backport release/v120
)