[ios] Various test and warning fixes for backwards compatibility #11196
Conversation
| if (@available(iOS 9.0, *)) { | ||
| XCTAssertEqualObjects([longFormatter stringFromCoordinate:coordinate], @"38 degrees, 54 minutes, and 48 seconds north by 77 degrees, 1 minute, and 57 seconds west"); | ||
| } else { | ||
| // Foundation in iOS 8 does not know how to pluralize coordinates. |
There was a problem hiding this comment.
The stringsdict format was introduced in iOS 7. I wonder if the problem is that Foundation.stringsdict is located in en.lproj whereas Foundation.strings is base-localized.
| @@ -373,7 +373,7 @@ class MGLDocumentationExampleTests: XCTestCase, MGLMapViewDelegate { | |||
| } | |||
| //#-end-example-code | |||
|
|
|||
| wait(for: [expectation], timeout: 1) | |||
| wait(for: [expectation], timeout: 5) | |||
There was a problem hiding this comment.
5 seconds is an awfully long time to render such a trivial local style that lacks any external resources. (MGLStyle.satelliteStreetsStyleURL() is a slight of hand: a nested class above interposes the method call, returning the one-liner style.)
There was a problem hiding this comment.
I agree that 5 seconds is a very long time-out, but CI is a hostile environment for timing-dependent tests — this can occasionally take up to 3-4 seconds to pass. 😞
There was a problem hiding this comment.
(Though I’m not ruling out some underlying issue, say, with the performance of the snapshotter.)
While #11007 will add an iOS 8–compatible conditional syntax, we’ll continue to support |
| @@ -67,7 +67,7 @@ - (void)testOrnamentPlacement { | |||
| CGFloat bottomSafeAreaInset = 0.0; | |||
| double accuracy = 0.01; | |||
|
|
|||
| if ( [self.mapView respondsToSelector:@selector(safeAreaInsets)] ) { | |||
| if (@available(iOS 11.0, *)) { | |||
There was a problem hiding this comment.
As an ObjC old-timer, changes like this always give me pause. I guess it's just the newer way of doing things, but I find the old explicit respondsToSelector: API availability check more comprehensible than the "what version are we running on" check. Informal protocols are a waning concept I guess...
Nothing to change here per se, just sharing my thoughts...
There was a problem hiding this comment.
Aye, this is definitely a bit of a shift in philosophy — I swapped to @available() here because -Wunguarded-availability-new will complain about just doing a selector-based check. We could certainly do both (and that’s technically safer, of course), but it feels redundant to me for modern system API.
There was a problem hiding this comment.
-Wunguarded-availability-newwill complain about just doing a selector-based check
Interesting. Sounds like a slow forced transition is underway, and we do need to nudge the codebase forward toward conformance around the newer pattern.
I agree with you re: redundancy, especially in tests... we should continue to keep an eye out for the right balance of style conformance and readability/comprehensibility, especially in the implementation.
Fixes: 'safeAreaInsets' is only available on iOS 11.0 or newer [-Wunguarded-availability-new]
Fixes: object of type 'MGLPointAnnotation/MGLPolyline *' is not compatible with array element type 'MGLShape<MGLFeature> *' [-Wobjc-literal-conversion]
Timing based tests are inherently flakey and prone to failure on slow CI: > Test case 'MGLDocumentationExampleTests.testMGLMapSnapshotter()' failed on 'iPhone X' (3.375 seconds) > Test case 'MGLDocumentationExampleTests.testMGLMapSnapshotter()' failed on 'iPhone 8' (3.413 seconds) > Test case 'MGLDocumentationExampleTests.testMGLMapSnapshotter()' failed on 'iPhone 7' (2.944 seconds)
…n iOS 8 Temporarily disable this test until iOS 8 compatibility is added.
friedbunny
force-pushed
the
fb-ios-test-and-warning-fixes
branch
from
b6acd0f to
63eb511
Compare
Fixes a couple build warnings and compatibility issues — see commit messages for the text of the warnings.
There’s still one broken test on iOS 8 in
-[MGLExpressionTests testConditionalExpressionObject], which will eventually be addressed by #11007.Unblocks #10742.
/cc @1ec5 @fabian-guerra @akitchen