Skip to content
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

Conversation

illuminati1911
Copy link
Collaborator

@illuminati1911 illuminati1911 commented Jan 7, 2025

Ground overlay iOS unit and integration tests + assert checks.

@jokerttu jokerttu force-pushed the feat/google_maps_flutter_ground_overlay_support_ios_tests branch from 9a513bc to 4745a71 Compare January 7, 2025 12:22
@illuminati1911 illuminati1911 force-pushed the feat/google_maps_flutter_ground_overlay_support_ios_tests branch from 5582bcc to 4555328 Compare January 9, 2025 02:28
@illuminati1911 illuminati1911 marked this pull request as ready for review January 9, 2025 02:28
@illuminati1911 illuminati1911 changed the title WIP test: add initial ios integration tests test: add initial ios integration tests Jan 9, 2025
Copy link

@jokerttu jokerttu left a 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';
Copy link

Choose a reason for hiding this comment

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

Are these imports needed?

Copy link
Collaborator Author

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) {
Copy link

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

Copy link
Collaborator Author

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) {
Copy link

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

Copy link
Collaborator Author

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;
Copy link

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

Copy link
Collaborator Author

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Restored to 60.

@@ -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.
Copy link

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.');
Copy link

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.');
Copy link

Choose a reason for hiding this comment

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

iOS

@jokerttu jokerttu merged commit 316f241 into feat/google_maps_flutter_ground_overlay_support Jan 9, 2025
1 check passed
@jokerttu jokerttu deleted the feat/google_maps_flutter_ground_overlay_support_ios_tests branch January 9, 2025 09:47
jokerttu added a commit that referenced this pull request Jan 9, 2025
* 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]>
jokerttu added a commit that referenced this pull request Jan 14, 2025
* 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants