Add/enhanced screenshots by jkmassel · Pull Request #9368 · wordpress-mobile/WordPress-Android

jkmassel · 2019-03-05T21:24:00Z

No description provided.

peril-wordpress-mobile · 2019-03-05T21:24:05Z

	Warnings
⚠️	PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

Use the new emulators and delete the old registry management code

jkmassel · 2019-03-21T22:07:45Z

Gemfile.lock

This is just for the purposes of reviewing this PR. Assuming we're happy with it, I can submit a new PR to adjust this for the tag, or can tag a new release in https://github.com/wordpress-mobile/release-toolkit

WordPress/src/androidTest/java/org/wordpress/android/ui/screenshots/WPScreenshotTest.java

Fix a merge issue Use the new screenshot filenames Ignore tmp folder Implement compositing adjustments Remove unused imports

JavonDavis · 2019-03-25T19:14:34Z

WordPress/src/androidTest/java/org/wordpress/android/ui/screenshots/WPScreenshotTest.java

I'm getting a compile error here, did you mean

Suggested change

this.__wpLogout();

this.wpLogout();

Ah – good catch! My bad.

That's fixed now, but the reason I brought in the old login / logout methods was because the new ones were failing for screenshots – in particular in other languages (Thai and Japanese seemed especially problematic). I spent some time with it and couldn't sort out why. I figured that could be something for a future PR?

WDYT?

Oh I meant to ask that, I'm not sure why they'd be failing, I actually didn't test on other languages so I'm not quite aware about what's happening, I'll look into it though thanks for pointing that out 👍🏽

Fix method names

Seems to make running the screenshot capture process less flaky

JavonDavis

@jkmassel this looks good to go to me! I left a few comments around the login steps. I did give it a run myself and got the following error though

[18:23:36]: fastlane finished with errors

[!] Tests failed for locale de-DE on device emulator-5554

I'm thinking this might be something I'm missing with my emulators though right?

JavonDavis · 2019-03-27T21:08:22Z

WordPress/src/androidTest/java/org/wordpress/android/ui/screenshots/WPScreenshotTest.java

@@ -59,65 +55,112 @@ public void wPScreenshotTest() {
        Screengrab.setDefaultScreenshotStrategy(new UiAutomatorScreenshotStrategy());

        wpLogin();


Small issue but this method doesn't check if the emulator isn't logged in already, is it worth doing that check here as well?

JavonDavis · 2019-03-27T21:13:06Z

WordPress/src/androidTest/java/org/wordpress/android/ui/screenshots/WPScreenshotTest.java

+    }

-        waitForRecyclerViewToStopReloading();
+    private void tmpWpLogout() {


I think this will have a problem if the app was already signed in with a self-hosted account. I had to handle that differently

WordPress-Android/WordPress/src/androidTest/java/org/wordpress/android/support/BaseTest.java

Lines 20 to 27 in 173d132

private void logout() {

boolean isSelfHosted = new MePage().go().isSelfHosted();

if (isSelfHosted) { // Logged in from self hosted connected

new MySitesPage().go().removeSite(E2E_SELF_HOSTED_USER_SITE_ADDRESS);

} else {

wpLogout();

}

}

For sure – as part of the screenshot generation, the device is wiped each time, so we don't need to worry about folks being signed in for this particular use case.

That said, probably a good case to handle for the general-purpose logout method.

Cool! That also makes my previous note void as well 🙂

JavonDavis

🚢

jtreanor · 2019-03-29T09:56:57Z

WordPress/src/androidTest/java/org/wordpress/android/support/WPSupportUtils.java

    // a different thread than the UI, the UI sometimes reports completion of an operation before repainting the
    // screen to reflect the change. Delaying by one frame ensures we're not taking a screenshot of a stale UI.
    public static void waitOneFrame() {
+    idleFor(17);


Maybe give this some indentation? 😄

jtreanor · 2019-03-29T09:57:47Z

WordPress/src/androidTest/java/org/wordpress/android/ui/screenshots/WPScreenshotTest.java


        // Click in the post title editor and ensure the caret is at the end of the title editor
-        focusEditPostTitle();
+//        focusEditPostTitle();


Why is this commented out? If its not needed anymore it would be nice to remove 👍

Good call – removed.

jtreanor · 2019-03-29T10:01:17Z

fastlane/SCREENSHOT_DEVICE_SETUP.md

-##### To adjust device parameters:
-
-Edit the device in Android Studio, and adjust to the specifications you'd like to use. Once you save the device, it'll make a new hardware profile with the settings you just provided. Right-click on that hardware profile, and choose "export". When merging, it can be useful to use the diff view to ensure that only the fields you meant to change are actually being changed – as a for-instance, Android Studio loves to try to overwrite your `sdcard.size`, and that change can almost always be discarded.
+Emulators can be entirely defined within their `.ini` file. Copying the `.ini` file into the correct directory (along with _another_ `.ini` file to point to it) is all that's needed – if it exists, the Android SDK will happily build a fresh new virtual device based on it.


It seems like the rebuild_screenshot_devices lane still exists for generating the devices. Is there more we can document about that here?

Sure can (and sure have) – let me know if there's anything else you have questions about, and I'm happy to write it up!

jkmassel force-pushed the add/enhanced-screenshots branch 3 times, most recently from 362a254 to 8168ca0 Compare March 19, 2019 04:01

jkmassel requested review from JavonDavis and jtreanor and removed request for JavonDavis March 19, 2019 04:01

jtreanor mentioned this pull request Mar 19, 2019

Add network mocking for end to end tests #9422

Merged

2 tasks

Update screenshot devices

d3a185e

Use the new emulators and delete the old registry management code

jkmassel added the releases label Mar 20, 2019

jkmassel assigned jkmassel and unassigned jkmassel Mar 20, 2019

jkmassel added this to the 12.1 milestone Mar 20, 2019

jkmassel force-pushed the add/enhanced-screenshots branch 3 times, most recently from 735f244 to 4dc1138 Compare March 21, 2019 22:06

jkmassel commented Mar 21, 2019

View reviewed changes

jkmassel force-pushed the add/enhanced-screenshots branch from 4dc1138 to d245068 Compare March 21, 2019 22:10

wpmobilebot reviewed Mar 21, 2019

View reviewed changes

WordPress/src/androidTest/java/org/wordpress/android/ui/screenshots/WPScreenshotTest.java Outdated Show resolved Hide resolved

wpmobilebot reviewed Mar 21, 2019

View reviewed changes

WordPress/src/androidTest/java/org/wordpress/android/ui/screenshots/WPScreenshotTest.java Outdated Show resolved Hide resolved

jkmassel added 3 commits March 21, 2019 16:15

Add enhanced screenshots

9ca2653

Fix a merge issue Use the new screenshot filenames Ignore tmp folder Implement compositing adjustments Remove unused imports

Adjust UI test idling for more stability

abca46f

Use Gutenberg for posts

1930ab6

jkmassel force-pushed the add/enhanced-screenshots branch 3 times, most recently from 01a02e9 to 193b572 Compare March 21, 2019 22:41

jkmassel mentioned this pull request Mar 22, 2019

Fix a bug in verify_diff for some diff strings wordpress-mobile/release-toolkit#36

Merged

jkmassel force-pushed the add/enhanced-screenshots branch from 6f60f27 to d10d321 Compare March 22, 2019 20:53

jkmassel marked this pull request as ready for review March 22, 2019 21:25

jkmassel mentioned this pull request Mar 22, 2019

Add the new StatsActivity to the locale manager #9443

Merged

JavonDavis reviewed Mar 25, 2019

View reviewed changes

jkmassel added 3 commits March 26, 2019 14:57

Use a local copy of login / logout for now

2eef727

Fix method names

Rebuild screenshot devices before capture

2b01f36

Seems to make running the screenshot capture process less flaky

Use the compositor integration branch

173d132

jkmassel force-pushed the add/enhanced-screenshots branch from d10d321 to 173d132 Compare March 26, 2019 20:57

JavonDavis reviewed Mar 27, 2019

View reviewed changes

JavonDavis approved these changes Mar 28, 2019

View reviewed changes

jtreanor reviewed Mar 29, 2019

View reviewed changes

jtreanor modified the milestones: 12.1 ❄️, 12.2 Apr 8, 2019

jkmassel added 2 commits April 9, 2019 14:17

Add additional screenshot device setup docs

f85f67c

Code cleanup

5ff1f3a

jkmassel merged commit 1f0e873 into develop Apr 9, 2019

jkmassel deleted the add/enhanced-screenshots branch April 9, 2019 20:43

		@@ -59,65 +55,112 @@ public void wPScreenshotTest() {
		Screengrab.setDefaultScreenshotStrategy(new UiAutomatorScreenshotStrategy());

		wpLogin();

	private void logout() {
	boolean isSelfHosted = new MePage().go().isSelfHosted();
	if (isSelfHosted) { // Logged in from self hosted connected
	new MySitesPage().go().removeSite(E2E_SELF_HOSTED_USER_SITE_ADDRESS);
	} else {
	wpLogout();
	}
	}

Conversation

jkmassel commented Mar 5, 2019

Uh oh!

peril-wordpress-mobile bot commented Mar 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JavonDavis left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JavonDavis Mar 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JavonDavis Mar 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JavonDavis Mar 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JavonDavis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

peril-wordpress-mobile bot commented Mar 5, 2019 •

edited

Loading

JavonDavis left a comment •

edited

Loading

JavonDavis Mar 27, 2019 •

edited

Loading

JavonDavis Mar 27, 2019 •

edited

Loading

JavonDavis Mar 28, 2019 •

edited

Loading