Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[ios, macos] Add style's text localization tests. #14430

Open
wants to merge 1 commit into
base: fabian-localize-14393
Choose a base branch
from

Conversation

fabian-guerra
Copy link
Contributor

@fabian-guerra fabian-guerra commented Apr 15, 2019

Adds localization tests to the style.

@fabian-guerra fabian-guerra added iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS tests labels Apr 15, 2019
@fabian-guerra fabian-guerra self-assigned this Apr 15, 2019
@fabian-guerra fabian-guerra force-pushed the fabian-style-localization-test branch from 358a9a8 to d0776df Compare April 15, 2019 18:19
@fabian-guerra fabian-guerra force-pushed the fabian-style-localization-test branch from d0776df to a68aa78 Compare April 15, 2019 18:53
@fabian-guerra fabian-guerra requested review from friedbunny and julianrex and removed request for julianrex April 15, 2019 19:27
@fabian-guerra fabian-guerra marked this pull request as ready for review April 15, 2019 19:29
@fabian-guerra fabian-guerra requested a review from 1ec5 as a code owner April 15, 2019 19:29
@fabian-guerra fabian-guerra requested a review from a team April 15, 2019 19:29
@@ -28,7 +28,7 @@ - (void)setUp {
[super setUp];

[MGLAccountManager setAccessToken:@"pk.feedcafedeadbeefbadebede"];
NSURL *styleURL = [[NSBundle bundleForClass:[self class]] URLForResource:@"one-liner" withExtension:@"json"];
NSURL *styleURL = [[NSBundle bundleForClass:[self class]] URLForResource:@"basic-style" withExtension:@"json"];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unlike one-liner.json, basic-style.json seems to have network dependencies. We’d need to mock those dependencies to ensure stable tests.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One possibility - we could (for the time being) convert this to an integration test, which do hit the network...

MGLAttributedExpression *attributedExpression = [MGLAttributedExpression attributedExpression:coalesceExpression attributes:@{}];
NSExpression *localizedExpression = [NSExpression mgl_expressionForAttributedExpressions:@[ [NSExpression expressionForConstantValue:attributedExpression] ]];
XCTAssertEqualObjects(countryLabel.text, localizedExpression);
countryLabel.text = [NSExpression expressionWithFormat:@"mgl_coalesce({%K, %K})", @"name_en", @"name"];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it’s more important to test that -[NSExpression mgl_expressionLocalizedIntoLocale:] returns the right value than to test the full integration with -[MGLStyle localizeLabelsIntoLocale:], especially considering the difficulty of removing network dependencies.

Copy link
Contributor

@julianrex julianrex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for walking me through it!

@stale stale bot added the archived Archived because of inactivity label Jun 14, 2019
@stale
Copy link

stale bot commented Jun 14, 2019

This pull request has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

@stale stale bot closed this Jun 14, 2019
@julianrex
Copy link
Contributor

@fabian-guerra is this closed?

@fabian-guerra
Copy link
Contributor Author

Is not @julianrex

@fabian-guerra fabian-guerra reopened this Jun 20, 2019
@stale stale bot removed the archived Archived because of inactivity label Jun 20, 2019
@stale stale bot added the archived Archived because of inactivity label Aug 19, 2019
@stale
Copy link

stale bot commented Aug 19, 2019

This pull request has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

@stale stale bot closed this Aug 19, 2019
@friedbunny friedbunny reopened this Aug 19, 2019
@stale stale bot removed the archived Archived because of inactivity label Aug 19, 2019
@friedbunny friedbunny removed their request for review October 23, 2019 20:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants