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

[ios] Test map view within tab bar controller #14704

Closed
wants to merge 14 commits into from

Conversation

jmkiley
Copy link
Contributor

@jmkiley jmkiley commented May 17, 2019

Adds tests for #14232 Part of ongoing work related to #14654,

I'm currently exploring how to detect that the map itself has become unresponsive. Currently planning to move the map after the timer has run for a few seconds, then check whether the map's center has changed.

cc @julianrex

@jmkiley jmkiley added iOS Mapbox Maps SDK for iOS tests labels May 17, 2019
@jmkiley jmkiley self-assigned this May 17, 2019
@jmkiley jmkiley changed the base branch from master to release-nectar May 17, 2019 22:48
@@ -84,6 +84,11 @@
"idiom" : "ipad",
"size" : "83.5x83.5",
"scale" : "2x"
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

to do: remove this.

}];
});

[self waitForExpectationsWithTimeout:10 handler:^(NSError * _Nullable error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI: timing based tests taking much, much longer than expected are why the Integration Tests target isn’t run on CI. Even though we’re not expecting to make these tests run on virtualized CI again, reducing their dependency on timing is always something to strive towards.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you! After talking to @julianrex, I'm going to take a different approach.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm still using the timer, but that is because I am not sure if there's another way to prevent the test from ending early.

@jmkiley
Copy link
Contributor Author

jmkiley commented May 21, 2019

It looks like MGLMapViewIntegrationTests already has some logic that I could reuse. I am going to expose -updateFromDisplayLink: and check for a delay of ~1 second.

@jmkiley jmkiley force-pushed the jmkiley-map-view-tests branch from 8506a76 to 58ce1ec Compare June 3, 2019 18:21
@jmkiley jmkiley changed the base branch from release-nectar to master June 3, 2019 18:21
@jmkiley
Copy link
Contributor Author

jmkiley commented Jun 3, 2019

I have rebased this on master and am almost ready for review. I am not sure if I should include platform/ios/ios.xcodeproj/xcshareddata/xcbaselines/ in this, or if that should be added to the .gitignore. @julianrex

@jmkiley jmkiley requested review from julianrex and friedbunny June 6, 2019 19:29
@jmkiley jmkiley marked this pull request as ready for review June 6, 2019 19:34
@jmkiley jmkiley requested a review from a team June 6, 2019 19:34
@@ -2992,40 +3006,40 @@
/* End PBXResourcesBuildPhase section */

/* Begin PBXShellScriptBuildPhase section */
074A7F0B2277BD67001A62D1 /* Insert Mapbox Access Token */ = {
07796BA2227908CB0059CAF1 /* Insert Mapbox Access Token */ = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

to do: I should remove this script, it doesn't matter 😅

@jmkiley
Copy link
Contributor Author

jmkiley commented Jun 6, 2019

Currently these tests pass on Simulator, but fail on a physical iPhone X.
Screenshot 2019-06-06 15 22 30

😅

@implementation MBXTestMapView

- (void)updateFromDisplayLink:(CADisplayLink*)displayLink {
[super updateFromDisplayLink:displayLink];
Copy link
Contributor

Choose a reason for hiding this comment

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

At the moment this isn't doing anything. This is what we need to time, and we should assert if the time diff is >= 1 second.

[self.window setRootViewController:(UITabBarController *)[storyboard instantiateViewControllerWithIdentifier:@"TabController"]];

// Access the tab controller from the story board, then access one of the childViewControllers.
self.tabController = (UITabBarController *)self.window.rootViewController;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd be tempted to set up tabController before you set the root view controller.

self.tabController = (UITabBarController *)self.window.rootViewController;
self.viewController = self.tabController.childViewControllers[0];

[self.viewController.view addSubview:self.mapView];
Copy link
Contributor

Choose a reason for hiding this comment

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

We might need to look at the view sizes, and potentially the resizing masks/constraints.

[self.mapView addAnnotation:annot];
}

- (void)testViewControllerNil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Separate tests are completely new instances, so I think you could move this test into the main one below.

XCTestExpectation *expectation = [self expectationWithDescription:@"Able to switch the tabs repeatedly without lag"];
__block NSInteger counter = 0;

__block NSTimer *repeatingTimer = [NSTimer scheduledTimerWithTimeInterval:0.1 repeats:YES block:^(NSTimer * _Nonnull timer) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's pair on this block - not sure what we need to do here.

counter++;
NSLog(@"%li", counter);
self.tabController.selectedIndex = counter % 2;
if (counter > 41) {
Copy link
Contributor

Choose a reason for hiding this comment

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

> 41? Is this a Douglas Adams reference??

repeatingTimer = nil;

MGLMapCamera *camera = [MGLMapCamera cameraLookingAtCenterCoordinate:CLLocationCoordinate2DMake(10, 10) altitude:100 pitch:0 heading:0];
[self.mapView flyToCamera:camera withDuration:4 completionHandler:^{
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this should happen at the start of the timer?

[expectation fulfill];
}];

[self measureBlock:^{
Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't need to call updateFromDisplayLink the waitForExpectations:.. below should handle that IIRC.

I hadn't considered using measureBlock - that's an interesting idea - but I don't think it's appropriate here.

@stale stale bot added the archived Archived because of inactivity label Aug 13, 2019
@stale
Copy link

stale bot commented Aug 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 Aug 14, 2019
@friedbunny friedbunny reopened this Aug 14, 2019
@stale stale bot removed the archived Archived because of inactivity label Aug 14, 2019
@friedbunny friedbunny removed their request for review October 23, 2019 20:54
@stale stale bot added the archived Archived because of inactivity label Mar 17, 2020
@stale
Copy link

stale bot commented Mar 17, 2020

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 Mar 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived Archived because of inactivity iOS Mapbox Maps SDK for iOS tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants