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

testDelegateRegionDidChange is too fragile #1360

Closed
kkaefer opened this issue Apr 28, 2015 · 4 comments
Closed

testDelegateRegionDidChange is too fragile #1360

kkaefer opened this issue Apr 28, 2015 · 4 comments

Comments

@kkaefer
Copy link
Contributor

kkaefer commented Apr 28, 2015

I'm seeing relatively frequent test failures, but they're not reproducible. It looks like they heavily depend on timing.

See https://travis-ci.org/mapbox/mapbox-gl-native/jobs/60350680#L1051

@incanus @1ec5

@friedbunny
Copy link
Contributor

Says it's doing two unanimated changes when it should only be performing one.

testDelegateRegionDidChange, ((unanimatedCount) equal to (1)) failed: ("2") is not equal to ("1") - regionDidChange delegate should indicate one unanimated change

test/ios/MapViewTests.m#L416

@friedbunny
Copy link
Contributor

I think L416's regionDidChange delegate should indicate one unanimated change is getting bogged down and executing after the next test case has started.

If that's correct, then this should fix it:

diff --git a/test/ios/MapViewTests.m b/test/ios/MapViewTests.m
index fff52ad..08a867d 100644
--- a/test/ios/MapViewTests.m
+++ b/test/ios/MapViewTests.m
@@ -406,7 +406,7 @@
                                                    tester.mapView.centerCoordinate = CLLocationCoordinate2DMake(0, 0);
                                                }];

-    [tester waitForTimeInterval:1];
+    [tester waitForAnimationsToFinishWithTimeout:1];

     XCTAssertEqual([notification.userInfo[@"animated"] boolValue],
                    NO,
@@ -415,6 +415,8 @@
                    1,
                    @"regionDidChange delegate should indicate one unanimated change");

+    [tester waitForTimeInterval:1];
+
     notification = [system waitForNotificationName:@"regionDidChangeAnimated"
                                             object:tester.mapView
                                whileExecutingBlock:^{

@friedbunny
Copy link
Contributor

Noooope, other way around. The test is failing because it's not waiting long enough for the environment to be reset and it's catching that.

@incanus
Copy link
Contributor

incanus commented May 7, 2015

This is looking great when testing locally and we can probably roll this into reenabling CI iOS tests in #1452. We'll give everything some care then to avoid false-positives like this one.

@incanus incanus closed this as completed May 7, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants