[Screenshots Automation] Fix locales switching #6834
Conversation
|
You can trigger optional UI/connected tests for these changes by visiting CircleCI here. |
| import com.woocommerce.android.util.WooLog | ||
| import leakcanary.AppWatcher | ||
|
|
||
| internal class LeakCanaryInstaller : ContentProvider() { |
There was a problem hiding this comment.
This follows the same logic as leak-canary's default artifact in using a ContentProvider for the initialization, the change is that we can have custom logic for the initialization now.
| @@ -20,7 +20,7 @@ object PackageUtils { | |||
| fun isTesting(): Boolean { | |||
| if (isTesting == null) { | |||
| isTesting = try { | |||
| Class.forName("com.woocommerce.android.viewmodel.BaseUnitTest") | |||
| Class.forName("org.junit.Test") | |||
There was a problem hiding this comment.
With this change, this function detects both UI tests and unit tests.
Generated by 🚫 dangerJS |
You can test the changes on this Pull Request by downloading an installable build, or scanning this QR code:
|
Codecov Report
@@ Coverage Diff @@
## trunk #6834 +/- ##
============================================
- Coverage 44.38% 44.37% -0.01%
Complexity 2972 2972
============================================
Files 535 536 +1
Lines 29598 29604 +6
Branches 3902 3902
============================================
Hits 13137 13137
- Misses 15243 15249 +6
Partials 1218 1218
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Huge thanks for working on the broken locales switching and the crash, @hichamboushaba!
From my side, I'm providing only the testing review, but not commenting on the code. I followed your suggestions here:
Confirm Locale switching works as expected
- Execute the following command: bundle exec fastlane take_screenshots
- Confirm the UI tests are executed and the device's language is changed for each iteration.
Confirm LeakCanary is initialized well for debug builds
- Launch a debug build of the app.
- Check logcat and confirm the presence of line: "Installing LeakCanary"
- The app no longer crashes while running the screenshots for the second time (for a different theme)
- Locales are switching:
- "Installing LeakCanary" is present in Logcat for debug build variant:
So, all these worked as expected 👍
I, however, saw several issues related to the screenshot test running under certain locales (Arabic, Japanese, ...), when the test was interrupted and failed. This is something I need to take a look into, but it has nothing to do with your changes.
|
Thanks @pachlava for the review.
We need to make changes to the test scenarios, as we are supposed to have different screenshots than the past ones p91TBi-9eM-p2, when I'll get to this step, I'll ask for your help 🙏. |
Got it @hichamboushaba 👍, feel free to ping me when the time comes 🙂 |
|
@0nko can you please check the code before merging this PR? 🙏. |
Part of #6833
Description
This PR fixes locale switching for screenshot generation, it adds the following changes to do so:
Regarding the last point, I asked in the LeakCanary repo about their opinion about integrating the same logic in the library itself, as the heap dumping ana analysis is already disabled by the library during tests, it's just that it attaches the watchers.
Testing instructions
Confirm Locale switching works as expected
bundle exec fastlane take_screenshotsConfirm LeakCanary is initialized well for debug builds
RELEASE-NOTES.txtif necessary.