-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Add Support for UI Testing the Jetpack app and Compose layouts #17869
Conversation
This is a workaround to support interacting with the compose ui from `LoginPrologueRevampedFragment`, which has an infinite animation. By disabling the auto-sync between tests and compose ui, we avoid `ComposeNotIdleException`. See: https://developer.android.com/jetpack/compose/testing#disable-autosync The change was added in the setup of `BaseTest` to support login in JP app tests.
I had to add instrumented test files for JP and WP build variants to successfully build ui tests for both apps. The requirement is caused by the string resource unique to the jetpack variant in the `LoginPrologueRevampedFragment`: `R.string.continue_with_wpcom_no_signup`. See https://developer.android.com/studio/test/advanced-test-setup This change affects UI tests in: - BlockEditorTests - ReaderTests - StatsTests - JPScreenshotTest - WPScreenshotTest
Both legacy login methods are no longer called from outside, so they can be private: - chooseContinueWithWpCom - chooseEnterYourSiteAddress
Moved both private methods under the public method which calls them
Support opening the signup magic link in the Jetpack app test by replacing the hard-coded "wordpress" scheme in the intent URI with `BuildConfig.FLAVOR_app`
Remove duplicated code in LoginFlow and SignupFlow by extracting the shared WP.com authentication logic to LandingPage
📲 You can test these changes on Jetpack by downloading jetpack-installable-build-pr17869-7a4ba84.apk
|
📲 You can test these changes on WordPress by downloading wordpress-installable-build-pr17869-7a4ba84.apk
|
Generated by 🚫 dangerJS |
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.
Thanks for adding the support for Compose on the UI tests @ovitrif!
The changes work well with the landing screen for Jetpack when tested locally. And CI WP tests are green 🎉
I did notice a few issues for some of the Jetpack tests when running locally, but I think those are out of the scope of this PR and can be fixed separately, as Jetpack tests are still local only at the moment.
I also have 1 suggestion I believe can improve the test in a separate comment, can you take a look if that is worth updating?
WordPress/src/androidTest/java/org/wordpress/android/e2e/pages/LandingPage.kt
Outdated
Show resolved
Hide resolved
@ParaskP7 I added you as reviewer mostly to validate the new dependencies: 92d5308 I noticed this commit from you: 85d3f05 (part of PR #17620) where you mentioned:
I'm not sure how to check if there's transitively used dependencies introduced by the newly added ones; and in case there are I'm thinking is best to declare them as well 🙇 Thanks in advance! |
👋 @ovitrif !
The dependencies you added looks good to me! 👍
Yes, you're right and thanks for noticing this intricacy with transitively used dependencies! 💯 Unfortunately, there is no quick way to check if there's transitively used dependencies that should be declared directly. If you still want to do that, which to be honest I don't recommend (as it will take you some time), the best way to do that you would be to follow this
If you do the above and run the
Let me know if all the above help and again, thank YOU for looking into that as well! 🥇 PS: At some point I want to add this Dependency Analysis Plugin to our project, but, I'll first want to get a 👍 👎 from WPAndroid engineers on this custom configuration on ignored variants. Then, I'll proceed with it. Hmmm, I am also now seeing that this PR has been merged and as per Tony's comment here, it seems that it is already in 1.18.0 (see custom configuration on ignored variants too, thanks Sergey indeed). So maybe, there is no need to add our custom configuration on ignored variants. I need to test that! 🤔 🎉 |
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.
Thank you for making this change and for considering using testTag
🙇 It's a ✅ for me from the test side of things!
Thank you @ParaskP7 for the great technical insight to help me check by myself the impact on transitive dependencies, you rock a million times 🏆 💯 💯 💯 🥇 I was able to run the Dependency Analysis Plugin, version 1.18.0 and configure it to target only one build variant & flavor mix 👍 After adding the plug-in I added this line to my
It only took 2m 51s to build 🎉 I don't see anything related to compose & testing in the File content
|
You rock @ovitrif ! ❤️
🎉 Woohoo 🎉
Coolio onwards and upwards! 🚀 |
Merging this PR, thank y'all for your help here 🙇 |
👋 @ovitrif !
Just a quick FYI that I did try that myself the other day and it seemed to be working as you described, it only took a couple of minutes on my PC as well. Because of that, today, I tried to add the FYI:
All the above, once more, made me stop from integrating the Btw Ovi, can you try that again on your side, after running |
@ParaskP7 It takes forever for me as well 😅 . Don't remember how I managed to get it run so fast back then but one thing I know for sure is that I didn't run gradle from command-line ( Although that as well takes forever and builds all variants now 😏 |
👋 @ovitrif ! Yeah, I know, it worked for me too, just this once, and then stopped! 🤷 😅 But, at least this non-working behavior is replicated between us two... I'll try that again at some point in the future. 🤞 Thanks for trying it out once more! 🥇 |
This PR:
To test:
jetpackWasabiDebug
WordPress/src/androidTest/java/org/wordpress/android/e2e
Regression Notes
Potential unintended areas of impact
N/a
What I did to test those areas of impact (or what existing automated tests I relied on)
Ran e2e and screenshots UI tests on:
LANDING_SCREEN_REVAMP
build config fieldfalse
LANDING_SCREEN_REVAMP
build config fieldtrue
What automated tests I added (or what prevented me from doing so)
N/a
PR submission checklist:
RELEASE-NOTES.txt
if necessary.