Add Localization UI Tests #1051

aa-stripe · 2018-11-29T22:41:49Z

Summary

Create a UI test that visits all the screens in the UI Examples app, and takes a screenshot of each one. These screenshots can then be gathered from DerivedData (or saved in a custom location using xcodebuild, to aid with localization QA, or even general QA)
Added a new UI Tests target to the UI Examples scheme.

Motivation

Automate Localization QA, and be able to scale the process as we add more languages.

Testing

Ran the UI tests with Cmd+u with the UI Examples scheme selected
Also ran on the command line with this xcodebuild invocation. This is the invocation that we should use in CI too:
xcodebuild clean test -workspace "Stripe.xcworkspace" -scheme "UI Examples" -configuration "Debug" -sdk "iphonesimulator" -destination "platform=iOS Simulator,name=iPhone 8 Plus,OS=11.2" -resultBundlePath "<result directory here>" -testLanguage "<language>" -testRegion "<region>"

A note for adding this to CI - we should put this in a for loop that goes through all the languages/regions, and then upload all the screenshots as artifacts in the CI run. A rough idea is:

for locale in [("de", "DE"), ("en", "US"), ("fr", "FR"), ... ]:
    language, region = locale
    output_dir = "{}_{}_{}".format(OUTPUT_PATH, language, region)
    run_command('xcodebuild clean test -workspace "Stripe.xcworkspace" -scheme "UI Examples" -configuration "Debug" -sdk "iphonesimulator" -destination "platform=iOS Simulator,name=iPhone 8 Plus,OS=11.2" -resultBundlePath "{}" -testLanguage "{}" -testRegion "{}"'.format(output_dir, language, region))

csabol-stripe · 2018-12-04T22:23:58Z

note there are known issues running this with 11.2 simulator (see: http://www.openradar.me/39348762 https://openradar.appspot.com/26320475). Running on 11.4 fixed for me

csabol-stripe · 2018-12-04T22:55:29Z

UI strings not covered by this scheme/script:
STPAddCardViewController:
- "Use shipping"
- "Use delivery"
- "Scan card"
- "OK"
STPPaymentMethodsViewController
- "Loading..."
STPShippingAddressViewController
- "Use Billing"
- "Invalid Shipping Address"
- "OK"
- "Delivery"
- "Contact"
- "Delivery Address"
STPPaymentCardTextField
- "ZIP"
- "Postal"
STPAddressFieldTableViewCell
(all of these except the last two might be actually covered when specififying a different region, I only ran en_US to test)
- "Province"
- "County"
- "State / Province / Region"
- "Postal code"
- "Country"
- "Email"
- "Phone"
STPPaymentMethodTableViewCell
- "%@ Ending In %@" (format string)
STPShippingMethodTableViewCell
- "Free"

@aa-stripe let me know how you would like to proceed. I think our options include adding some additional interactions to the script to hit some of these, creating a new scheme that explicitly hits all of these, add test schemes to Standard/Custom Integration targets to try to hit these, something else?

aa-stripe · 2018-12-05T00:27:14Z

Per offline discussion, @csabol-stripe to take the action of adding the remaining screens/strings not covered by my test to the example app. I will provide any support as necessary.

This reverts commit eecfa9a.

csabol-stripe · 2019-02-07T23:30:02Z

Adding @danj-stripe as a reviewer. This patch is getting quite large as I'm also incorporating updated translations from QA (since they are based on and need another round trip of screenshots). Once I get the all clear from loc qa I can try to at least separate this out into a strings update patch and ui-testing/screenshot generating patch

danj-stripe

Couple questions/nits

danj-stripe · 2019-02-08T18:22:31Z

ci_scripts/screenshot_localization_tester.sh

+  #statements
+  IFS=",";
+  set -- $i;
+  xcodebuild clean test -workspace "${script_dir}/../Stripe.xcworkspace" -scheme "LocalizationTester" -configuration "Debug" -sdk "iphonesimulator" -destination "platform=iOS Simulator,name=iPhone 6s,OS=11.4" -resultBundlePath "$HOME/Desktop/loc_qa/$1_$2" -testLanguage $1 -testRegion $2


I think it's probably better to keep this inside the repository, instead of putting it on the desktop. Maybe in the build/ directory

makes sense!

danj-stripe · 2019-02-08T18:29:28Z

LocalizationTester/MockCustomerContext.m

+    if ([source isKindOfClass:[STPToken class]] && ((STPToken *)source).card != nil) {
+        [_mockCustomer.mockSources addObject:((STPToken *)source).card];
+    }
+    completion(nil);


3 of the methods that end up calling completion make sure it's not nil first, but this & selectDefaultCustomerSource don't. Should they?

yup, thanks

danj-stripe · 2019-02-08T18:40:24Z

LocalizationTesterUITests/LocalizationTesterUITests.m

+    XCUIApplication *app = [[XCUIApplication alloc] init];
+    XCUIElementQuery *tablesQuery = app.tables;
+
+#pragma mark -


My understanding of #pragma mark is that it puts things into the jump bar, and that's the way I've always seen it used. All of these plain separators look strange. IMO you should include some text if you're going to use it (like the comments that you have on the next lines)

blegh yeah, I'll add text

danj-stripe · 2019-02-08T18:51:34Z

LocalizationTesterUITests/LocalizationTesterUITests.m

+}
+
+- (void)_waitForElementToAppear:(XCUIElement *)element {
+    XCTAssert([element waitForExistenceWithTimeout:5], "An exepected element did not appear on screen: \(element)");


afaik this only works in swift, but I'm pretty sure the string can be a format string.

Suggested change

XCTAssert([element waitForExistenceWithTimeout:5], "An exepected element did not appear on screen: \(element)");

XCTAssert([element waitForExistenceWithTimeout:5], "An expected element did not appear on screen: %@", element);

copy-pasta :P

danj-stripe · 2019-02-08T18:52:36Z

LocalizationTests/LocalizationTests.m

+    [super tearDown];
+}
+
+- (void)testExample {


Should this file be deleted? Or is it going to have some content later?

should be deleted. Thanks

danj-stripe · 2019-02-08T18:54:32Z

Stripe/STPAddCardViewController.m

@@ -48,6 +48,8 @@ @interface STPAddCardViewController ()<
    UITableViewDelegate,
    UITableViewDataSource>

+@property (nonatomic) BOOL alwaysShowScanCardButton;
+@property (nonatomic) BOOL alwaysEnableDoneButton;


My objc is rusty, but do you need these duplicated here if they're declared in the private header?

Maybe the private header needs to be imported?

I'm a bit confused about why this is needed as well, but _alwaysShowScanCardButton is not a valid ivar without it. Also matches the pattern of the other properties declared in the private header

yuki-stripe

Last 3 commits look good!

aa-stripe · 2019-03-19T17:19:23Z

@csabol-stripe @danj-stripe @yuki-stripe thank you so much for taking this to the finish line!

- Refactors the `containerView` from `DocumentScanningView` into `CameraPreviewContainerView` so it can be shared between document and selfie - Adds a new `SelfieScanningView` to display a camera preview or scanned image

aa-stripe added 4 commits November 29, 2018 12:36

Add a unit test that visits all screens in the Examples test app

ee2b9f4

Clean up some unused code, change shipping form test data

318f1d3

Remove crappy Xcode UI test tokens

eaffa83

Change file name in comment

8670e25

aa-stripe assigned csabol-stripe Nov 29, 2018

csabol-stripe assigned aa-stripe Dec 4, 2018

aa-stripe removed their assignment Dec 5, 2018

csabol-stripe added 12 commits December 14, 2018 09:13

STPAddCardViewController strings all covered.

30cc38a

Didn't need those. STPPaymentCardTextField covered

5341802

Actually now all of STPAddCardViewController is covered

b2128b3

STPPaymentMethodsViewController covered.

893c1c6

All screens available, just need to visit them.

8c9841a

Full coverage in tests

ad2bbd6

Autoscroll

be680d8

Starting reverts

eecfa9a

Revert "Starting reverts"

5882b6b

This reverts commit eecfa9a.

Reverts unneeded changes

b9688af

Suppress autoscrolling as needed

c444be6

Add localizations to LocalizationTester

0e02684

csabol-stripe assigned aa-stripe and unassigned csabol-stripe Dec 20, 2018

csabol-stripe added 6 commits February 6, 2019 15:02

Adds screenshot script.

a276eec

Updated translations.

6b39815

Move the script.

88c6c3f

Permissions

342b4f9

Suppress keyboard.

cd246b6

Use correct language codes

2dd45d9

csabol-stripe requested a review from danj-stripe February 7, 2019 23:28

csabol-stripe assigned danj-stripe Feb 7, 2019

danj-stripe reviewed Feb 8, 2019

View reviewed changes

danj-stripe assigned csabol-stripe Feb 8, 2019

Updates from review

a5494fd

csabol-stripe removed their assignment Feb 8, 2019

danj-stripe approved these changes Feb 11, 2019

View reviewed changes

stripe-ci added the approved label Feb 11, 2019

stripe-ci unassigned danj-stripe Feb 11, 2019

csabol-stripe added 2 commits March 18, 2019 13:58

Merge branch 'master' into aa-stripe/add-localization-tests

1586497

Pulls latest translations, fixes es-AR script issue.

b4d9005

stripe-ci removed the approved label Mar 18, 2019

csabol-stripe added 2 commits March 18, 2019 14:30

Fixes string file encoding. Payment method -> payment options

fb061b3

Update reference snapshots for new translations

2961ea0

csabol-stripe assigned csabol-stripe and unassigned aa-stripe Mar 18, 2019

csabol-stripe requested a review from danj-stripe March 18, 2019 23:31

csabol-stripe assigned danj-stripe and unassigned csabol-stripe Mar 18, 2019

yuki-stripe approved these changes Mar 19, 2019

View reviewed changes

stripe-ci added the approved label Mar 19, 2019

stripe-ci assigned aa-stripe and unassigned danj-stripe Mar 19, 2019

csabol-stripe merged commit 1c5505f into stripe:master Mar 19, 2019

csabol-stripe deleted the aa-stripe/add-localization-tests branch March 19, 2019 17:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Localization UI Tests #1051

Add Localization UI Tests #1051

aa-stripe commented Nov 29, 2018

csabol-stripe commented Dec 4, 2018

csabol-stripe commented Dec 4, 2018

aa-stripe commented Dec 5, 2018

csabol-stripe commented Feb 7, 2019

danj-stripe left a comment

danj-stripe Feb 8, 2019

csabol-stripe Feb 8, 2019

danj-stripe Feb 8, 2019

csabol-stripe Feb 8, 2019

danj-stripe Feb 8, 2019

csabol-stripe Feb 8, 2019

danj-stripe Feb 8, 2019

csabol-stripe Feb 8, 2019

danj-stripe Feb 8, 2019

csabol-stripe Feb 8, 2019

danj-stripe Feb 8, 2019

csabol-stripe Feb 8, 2019

yuki-stripe left a comment

aa-stripe commented Mar 19, 2019

	XCTAssert([element waitForExistenceWithTimeout:5], "An exepected element did not appear on screen: \(element)");
	XCTAssert([element waitForExistenceWithTimeout:5], "An expected element did not appear on screen: %@", element);

Add Localization UI Tests #1051

Add Localization UI Tests #1051

Conversation

aa-stripe commented Nov 29, 2018

Summary

Motivation

Testing

csabol-stripe commented Dec 4, 2018

csabol-stripe commented Dec 4, 2018

aa-stripe commented Dec 5, 2018

csabol-stripe commented Feb 7, 2019

danj-stripe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuki-stripe left a comment

Choose a reason for hiding this comment

aa-stripe commented Mar 19, 2019