Use built-in expectation functionality to replace performSelector dance.

mapbox · Apr 24, 2018 · 235f24f · 235f24f
1 parent 438243b
commit 235f24f
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 22 deletions.
diff --git a/platform/ios/CHANGELOG.md b/platform/ios/CHANGELOG.md
@@ -11,6 +11,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT
 ### Other
 
 * Fixed an issue where `-[MGLMapView metersPerPixelAtLatitude:]` was removed, but not marked as unavailable. ([#11765](https://github.com/mapbox/mapbox-gl-native/pull/11765))
+* Adjusted when and how the camera transition update and finish callbacks are called, fixing recursion bugs. ([#5833](https://github.com/mapbox/mapbox-gl-native/issues/5833), [#11180](https://github.com/mapbox/mapbox-gl-native/issues/11180))
 
 ## 4.0.0 - April 19, 2018
 

diff --git a/platform/ios/Integration Tests/MGLCameraTransitionTests.mm b/platform/ios/Integration Tests/MGLCameraTransitionTests.mm
@@ -9,67 +9,59 @@ @implementation MBCameraTransitionTests
 - (void)testSetAndResetNorthWithDispatchAsyncInDelegateMethod {
 
     XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"];
+    expectation.expectedFulfillmentCount = 2;
+    expectation.assertForOverFulfill = YES;
 
     __weak typeof(self) weakself = self;
-    __block NSInteger delegateCallCount = 0;
 
     self.regionDidChange = ^(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated) {
 
         MBCameraTransitionTests *strongSelf = weakself;
 
         if (!strongSelf) return;
 
-        delegateCallCount++;
+        [expectation fulfill];
 
         MGLTestAssert(strongSelf, mapView.userTrackingMode != MGLUserTrackingModeFollowWithHeading);
         if (mapView.direction != 0.0) {
             dispatch_async(dispatch_get_main_queue(), ^{
                 [mapView resetNorth];
             });
         }
-
-        [NSObject cancelPreviousPerformRequestsWithTarget:expectation selector:@selector(fulfill) object:nil];
-        [expectation performSelector:@selector(fulfill) withObject:nil afterDelay:0.5];
     };
 
     [self.mapView setDirection:90 animated:YES];
 
     // loop, render, and wait
     [self waitForExpectations:@[expectation] timeout:1.5];
-
-    XCTAssert(delegateCallCount == 2, @"Expecting 2 regionDidChange callbacks, got %ld", delegateCallCount); // Once for the setDirection and once for the reset north
 }
 
 
 - (void)testSetAndResetNorthInDelegateMethod {
 
     XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"];
+    expectation.expectedFulfillmentCount = 2;
+    expectation.assertForOverFulfill = YES;
 
     __weak typeof(self) weakself = self;
-    __block NSInteger delegateCallCount = 0;
 
     self.regionDidChange = ^(MGLMapView *mapView, MGLCameraChangeReason reason, BOOL animated) {
 
         MBCameraTransitionTests *strongSelf = weakself;
 
         if (!strongSelf) return;
 
-        delegateCallCount++;
+        [expectation fulfill];
 
         MGLTestAssert(strongSelf, mapView.userTrackingMode != MGLUserTrackingModeFollowWithHeading);
         if (mapView.direction != 0.0) {
             NSLog(@"Reset to north");
             [mapView resetNorth];
         }
-
-        [NSObject cancelPreviousPerformRequestsWithTarget:expectation selector:@selector(fulfill) object:nil];
-        [expectation performSelector:@selector(fulfill) withObject:nil afterDelay:0.5];
     };
 
     [self.mapView setDirection:90 animated:YES];
     [self waitForExpectations:@[expectation] timeout:1.5];
-
-    XCTAssert(delegateCallCount == 2, @"Expecting 2 regionDidChange callbacks, got %ld", delegateCallCount); // Once for the setDirection and once for the reset north
 }
 
 
@@ -111,6 +103,8 @@ - (void)testSetCenterCancelsTransitions {
 - (void)testSetCenterCoordinateInDelegateMethod {
 
     XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"];
+    expectation.expectedFulfillmentCount = 2;
+    expectation.assertForOverFulfill = YES;
 
     __weak typeof(self) weakself = self;
     __block NSInteger delegateCallCount = 0;
@@ -177,15 +171,12 @@ - (void)testSetCenterCoordinateInDelegateMethod {
 
         delegateCallCount++;
 
-        [NSObject cancelPreviousPerformRequestsWithTarget:expectation selector:@selector(fulfill) object:nil];
-        [expectation performSelector:@selector(fulfill) withObject:nil afterDelay:0.5];
+        [expectation fulfill];
     };
 
     // Should take MGLAnimationDuration seconds (0.3)
     [self.mapView setCenterCoordinate:target zoomLevel:15.0 animated:YES];
     [self waitForExpectations:@[expectation] timeout:1.5];
-
-    XCTAssert(delegateCallCount == 2, @"Expecting 2 regionDidChange callbacks, got %ld", delegateCallCount); // Once for the setDirection and once for the reset north
 }
 
 - (void)testFlyToCameraInDelegateMethod {
@@ -194,6 +185,8 @@ - (void)testFlyToCameraInDelegateMethod {
 
     __weak typeof(self) weakself = self;
     __block NSInteger delegateCallCount = 0;
+    expectation.expectedFulfillmentCount = 3;
+    expectation.assertForOverFulfill = YES;
 
     CLLocationCoordinate2D target = CLLocationCoordinate2DMake(40.0, 40.0);
     CLLocationCoordinate2D target2 = CLLocationCoordinate2DMake(30.0, 30.0);
@@ -277,6 +270,8 @@ - (void)testFlyToCameraInDelegateMethod {
         }
 
         delegateCallCount++;
+
+        [expectation fulfill];
     };
 
     // Should take MGLAnimationDuration

diff --git a/platform/ios/Integration Tests/MGLShapeSourceTests.m b/platform/ios/Integration Tests/MGLShapeSourceTests.m
@@ -36,6 +36,8 @@ - (void)testSettingShapeSourceToNilInRegionDidChange {
     [self.style addLayer:layer];
 
     XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"];
+    expectation.expectedFulfillmentCount = 1;
+    expectation.assertForOverFulfill = YES;
 
     __weak typeof(self) weakself = self;
     __block NSInteger delegateCallCount = 0;
@@ -62,8 +64,7 @@ - (void)testSettingShapeSourceToNilInRegionDidChange {
             shapeSource.shape = nil;
         }
 
-        [NSObject cancelPreviousPerformRequestsWithTarget:expectation selector:@selector(fulfill) object:nil];
-        [expectation performSelector:@selector(fulfill) withObject:nil afterDelay:0.5];
+        [expectation fulfill];
     };
 
     // setCenterCoordinate is NOT animated here.
@@ -94,6 +95,8 @@ - (void)testSettingShapeSourceToNilInRegionIsChanging {
     [self.style addLayer:layer];
 
     XCTestExpectation *expectation = [self expectationWithDescription:@"regionDidChange expectation"];
+    expectation.expectedFulfillmentCount = 1;
+    expectation.assertForOverFulfill = YES;
 
     __block NSInteger delegateCallCount = 0;
     __weak typeof(self) weakself = self;
@@ -111,8 +114,7 @@ - (void)testSettingShapeSourceToNilInRegionIsChanging {
             MGLTestFail(weakself);
         }
 
-        [NSObject cancelPreviousPerformRequestsWithTarget:expectation selector:@selector(fulfill) object:nil];
-        [expectation performSelector:@selector(fulfill) withObject:nil afterDelay:0.5];
+        [expectation fulfill];
     };
 
     // Should take MGLAnimationDuration seconds (0.3)

diff --git a/platform/macos/CHANGELOG.md b/platform/macos/CHANGELOG.md
@@ -6,6 +6,11 @@
 
 * Deprecated `+[NSExpression featurePropertiesVariableExpression]` use `+[NSExpression featureAttributesVariableExpression]` instead. ([#11748](https://github.com/mapbox/mapbox-gl-native/pull/11748))
 
+### Other changes
+
+* Adjusted when and how the camera transition update and finish callbacks are called, fixing recursion bugs. ([#5833](https://github.com/mapbox/mapbox-gl-native/issues/5833), [#11180](https://github.com/mapbox/mapbox-gl-native/issues/11180))
+
+
 ## 0.7.0 - April 19, 2018
 
 The 0.7._x_ series of releases will be the last to support macOS 10.10. The minimum macOS deployment version will increase to macOS 10.11.0 in a future release.