-
Notifications
You must be signed in to change notification settings - Fork 295
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
Fix UI Tests #1345
Fix UI Tests #1345
Conversation
* fix constraints warning by adding leading and trailing constraints for nested stackViews * remove print statement
@@ -20,7 +20,7 @@ jobs: | |||
- name: Install CocoaPod dependencies | |||
run: pod install | |||
- name: Run Unit Tests | |||
run: set -o pipefail && xcodebuild -workspace 'Braintree.xcworkspace' -sdk 'iphonesimulator' -configuration 'Debug' -scheme 'UnitTests' -destination 'name=iPhone 14,OS=17.2,platform=iOS Simulator' test | ./Pods/xcbeautify/xcbeautify | |||
run: set -o pipefail && xcodebuild -workspace 'Braintree.xcworkspace' -sdk 'iphonesimulator' -configuration 'Debug' -scheme 'UnitTests' -destination 'name=iPhone 15,OS=17.2,platform=iOS Simulator' test | ./Pods/xcbeautify/xcbeautify |
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 is pulled from the findings in PR #1328
@@ -7,5 +7,10 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate { | |||
func scene(_ scene: UIScene, openURLContexts URLContexts: Set<UIOpenURLContext>) { | |||
AppSwitcher.openVenmoURL = URLContexts.first?.url | |||
} | |||
|
|||
func scene(_ scene: UIScene, willConnectTo session: UISceneSession, options connectionOptions: UIScene.ConnectionOptions) { |
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 is pulled from the findings in PR #1290
app.enterCardDetailsWith(cardNumber: "4000000000001075") | ||
app.tokenizeButton.tap() | ||
sleep(2) | ||
|
||
waitForElementToAppear(app.liabilityCouldNotBeShiftedMessage, timeout:30) | ||
waitForElementToAppear(app.liabilityCouldNotBeShiftedMessage, timeout:45) |
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 was failing because the Cardinal timeout for this card number is 30s, so I bumped it a bit beyond to ensure we always get a response
@@ -992,6 +992,7 @@ | |||
"$(inherited)", | |||
"@executable_path/Frameworks", | |||
"@loader_path/Frameworks", | |||
"$(FRAMEWORK_SEARCH_PATHS)", |
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.
I found a few threads in the GitHub actions issues that indicated that at times the CI runner cannot find the frameworks, so adding this to the demo app should increase the search path coverage in addition to the above paths.
@@ -12,11 +12,10 @@ class ThreeDSecure_V2_UITests: XCTestCase { | |||
app.launchArguments.append("-UITestHardcodedClientToken") | |||
app.launchArguments.append("-Integration:ThreeDSecureViewController") | |||
app.launch() | |||
|
|||
waitForElementToAppear(app.cardNumberTextField) |
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.
Moving this logic out of setup produced more stability in the tests both locally and in CI.
@@ -19,7 +19,7 @@ class AmericanExpress_UITests: XCTestCase { | |||
app.buttons["Valid card"].tap() | |||
sleep(2) | |||
|
|||
XCTAssertTrue(app.buttons["45256433 Points, 316795.03 USD"].waitForExistence(timeout: 10)) | |||
XCTAssertTrue(app.buttons["45256433 Points, 316795.03 USD"].waitForExistence(timeout: 20)) |
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.
While there are several instances in this PR just bumping timeouts, several of these test were also hitting the same timeouts locally. If the test completes before time timeout is hit it'll move on to the next test so there is no real harm in bumping the timeouts for the recurring failing tests.
I have run the UI tests 10x between Friday and today and all 10 runs have passed. You can see this under the "checks" tab. |
Summary of changes
scene(_:willConnectTo:options:)
to the MockVenmo App #1290Checklist
[ ] Added a changelog entryAuthors