-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Cover more user paths where UI tests ask to save password #20751
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
Conversation
|
| App Name | WordPress Alpha |
|
| Configuration | Release-Alpha | |
| Build Number | pr20751-eca05e6 | |
| Version | 22.6 | |
| Bundle ID | org.wordpress.alpha | |
| Commit | eca05e6 | |
| App Center Build | WPiOS - One-Offs #5944 |
bc849ed to
c5771cf
Compare
|
| App Name | Jetpack Alpha |
|
| Configuration | Release-Alpha | |
| Build Number | pr20751-eca05e6 | |
| Version | 22.6 | |
| Bundle ID | com.jetpack.alpha | |
| Commit | eca05e6 | |
| App Center Build | jetpack-installable-builds #4972 |
27ab2b3 to
eca05e6
Compare
Generated by 🚫 dangerJS |
| // alert where "Save Password" is. | ||
| app.buttons["Not Now"].tap() | ||
| } | ||
| app.dismissSavePasswordPrompt() |
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.
We are calling the new dismissSavePasswordPrompt() logic in three different places.
There might be a more centralized location for this. At the moment, I'm more interested in getting the builds to a stable place. We can refactor later.
jostnes
left a comment
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 probably need to start using 14.3 locally to start seeing these issues 😅
i had a look at the logs of the failing builds and it seems like the failures are all related to the save password prompt (notice how the prompt is there on different screens, i too am not sure where the ideal location for this dismissal is for now...):
calling dismissSavePasswordPrompt() to cover more login paths look to be a good solution in this case 👍
|
Thanks for the quick review @jostnes. Unfortunately, this still doesn't address all our issues 😞 I have a build that uses these changes and still got the failed to launch errors. Oh well... one step at a time, I guess. |
|
@mokagio, these launch errors are not making tests to fail. As I mentioned here, they are related to removing the app after each test which was introduced here, here's a build. As an example, here's how the logs looked like before that. The
Using the build you've mentioned as reference, the xcresult file available in Artifacts shows only 2 failed tests, with 3 failed executions each, while all the other passed in the first try.
The two failed tests failed with regular timeouts, so nothing to do with app not being launched. |
|
Thanks @tiagomar for the detailed info 🙌 🤔 I'm not sure then why Test Analytics is picking them up as the failure message. Maybe something to do with how XCTest logs errors etc. 🤷♂️ |







@jostnes and @tiagomar have done great work recently tidying up UI tests. Still, I keep experiencing CI failures. Maybe I'm jinxed 😅
For example, I was doing experiments to try understand what could have introduced all the "could not launch app" CI logs and I got a string of failures. These changes were the only ones that resulted in a green build.
Note: I didn't try more launches. Given I've only experienced failures recently, this seems like an improvement already.
Keen to hear what you think!