-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: add initial ios integration tests #12
test: add initial ios integration tests #12
Conversation
9a513bc
to
4745a71
Compare
5582bcc
to
4555328
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
Only position path for unit tests missing.
Added few nits.
@@ -4,6 +4,8 @@ | |||
|
|||
import 'dart:async'; | |||
import 'dart:convert'; | |||
import 'dart:developer'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these imports needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope. Removed.
expect(response.zIndex, source.zIndex); | ||
|
||
// Only iOS supports zoomLevel | ||
if (Platform.isIOS) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unnecessary test.
these tests are always runend under ios
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed and also removed from Android.
expect(response.zoomLevel, source.zoomLevel); | ||
} | ||
// Only Android (using position) and iOS supports `anchor` | ||
if ((Platform.isAndroid && source.position != null) || Platform.isIOS) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary condition here as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed and also removed from Android.
@@ -3,7 +3,7 @@ | |||
archiveVersion = 1; | |||
classes = { | |||
}; | |||
objectVersion = 60; | |||
objectVersion = 54; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what causes this. could this change be removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is generated by Xcode based on the xcode version. It tells the project the format version of the project files. Which Xcode are you using? I'm not completely sure if it's safe to change it. We could try.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restored to 60.
...ogle_maps_flutter_ios/example/ios14/ios/RunnerTests/GoogleMapsGroundOverlayControllerTests.m
Outdated
Show resolved
Hide resolved
@@ -41,4 +51,7 @@ | |||
|
|||
/// Returns true if a ground overlay with the given identifier exists on the map. | |||
- (bool)hasGroundOverlaysWithIdentifier:(NSString *)identifier; | |||
|
|||
/// Returns FGMPlatformGroundOverlay for id. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit "for identifier"
(GroundOverlay groundOverlay) => | ||
groundOverlay.position == null || | ||
groundOverlay.zoomLevel != null), | ||
'On ios zoom level must be set when position is set for ground overlays.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iOS
assert( | ||
mapObjects.groundOverlays.every((GroundOverlay groundOverlay) => | ||
groundOverlay.position == null || groundOverlay.zoomLevel != null), | ||
'On ios zoom level must be set when position is set for ground overlays.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iOS
316f241
into
feat/google_maps_flutter_ground_overlay_support
* test: add initial ios integration tests * fix integration tests for iOS * fix: add zoom level assert to ios groundoverlays * test: ios ground overlay unit tests * test: refactor integration tests * refactor: pr fixes and refactoring --------- Co-authored-by: Joonas Kerttula <[email protected]>
* test: add initial ios integration tests * fix integration tests for iOS * fix: add zoom level assert to ios groundoverlays * test: ios ground overlay unit tests * test: refactor integration tests * refactor: pr fixes and refactoring --------- Co-authored-by: Joonas Kerttula <[email protected]>
Ground overlay iOS unit and integration tests + assert checks.