[ios] add unit test target #7740

kirylkaveryn · 2024-03-30T09:19:11Z

kirylkaveryn commented

2024-03-30 09:19:11 +00:00

This PR adds the Unit test target to iOS.

How to add unit tests

The easiest way to add some tests:

select in the XCode File -> New -> File...
find and select Unit Test Case Class
pay attention that OMapsTest target is selected
Save into Tests directory
Add the line @testable import Organic_Maps__Debug_ below import XCTest to get access to all OM public and internal classes.
Write your tests
Select a simulator as a run destination to run test cases.

Example of use

As an example the test case for the CarPlayService method createEstimates (thanks to @fabwu for fix) was added:

Before the fix we can see that test case the with coma-separator used for the distace initialization fails and with dot passes:

After fix (using the double instead of the string):

Important

At least one issue should be resolved to merge this PR:
When we run testcases and they are finished, the app fails with error in the SharedBufferManager:

You still can see passed/failed tests but error is annoying. This bug may affect async tests.

This PR adds the `Unit test target` to iOS. --- ### How to add unit tests The easiest way to add some tests: 1. select in the XCode `File -> New -> File...` 2. find and select `Unit Test Case Class` 3. pay attention that `OMapsTest` target is selected <img width="350" alt="image" src="https://github.com/organicmaps/organicmaps/assets/79797627/f77b5030-0cbe-42e9-a6d2-a88f38fd4002"> 4. Save into `Tests` directory 5. Add the line `@testable import Organic_Maps__Debug_` below `import XCTest` to get access to all OM public and internal classes. 6. Write your tests 7. Select a simulator as a run destination to run test cases. --- ### Example of use As an example the test case for the `CarPlayService` method `createEstimates` (thanks to @fabwu for fix) was added: Before the fix we can see that test case the with coma-separator used for the distace initialization fails and with dot passes: <img width="1340" alt="image" src="https://github.com/organicmaps/organicmaps/assets/79797627/6d211136-89d5-4dc2-b706-37f7bc12a5ec"> After fix (using the double instead of the string): <img width="851" alt="image" src="https://github.com/organicmaps/organicmaps/assets/79797627/35ee076b-8a0b-4037-9449-df956f456467"> --- ### Important At least one issue should be resolved to merge this PR: When we run testcases and they are finished, the app fails with error in the `SharedBufferManager`: <img width="1728" alt="image" src="https://github.com/organicmaps/organicmaps/assets/79797627/fcb19181-1c36-47f1-b521-e2d1265d1e53"> You still can see passed/failed tests but error is annoying. This bug may affect async tests.

biodranik (Migrated from github.com) reviewed 2024-04-09 18:06:23 +00:00

iphone/Maps/Classes/EAGLView.mm

biodranik (Migrated from github.com) commented

2024-04-09 17:59:25 +00:00

It would be clearer to disable this call where it has originated:

- (void)viewDidLayoutSubviews {
  [super viewDidLayoutSubviews];
  if (!self.mapView.drapeEngineCreated)
    [self.mapView createDrapeEngine];
}

  override func viewDidLayoutSubviews() {
    super.viewDidLayoutSubviews()
    if mapView?.drapeEngineCreated == false {
      mapView?.createDrapeEngine()
    }
    updateVisibleViewPortState(viewPortState)
  }

It would be clearer to disable this call where it has originated: ``` - (void)viewDidLayoutSubviews { [super viewDidLayoutSubviews]; if (!self.mapView.drapeEngineCreated) [self.mapView createDrapeEngine]; } override func viewDidLayoutSubviews() { super.viewDidLayoutSubviews() if mapView?.drapeEngineCreated == false { mapView?.createDrapeEngine() } updateVisibleViewPortState(viewPortState) } ```

iphone/Maps/Classes/MapsAppDelegate.h

biodranik (Migrated from github.com) commented

2024-04-09 18:00:06 +00:00

Maybe like this?

-(BOOL)isDrapeDisabled;

Maybe like this? ```suggestion -(BOOL)isDrapeDisabled; ```

iphone/Maps/Maps.xcodeproj/project.pbxproj

					
				@ -4598,1 +4664,4 @@

						};

						ED097E7C2BB80C330006ED01 /* Debug */ = {

							isa = XCBuildConfiguration;

							baseConfigurationReference = 347526FA1DC0B00F00918CF5 /* common-debug.xcconfig */;

biodranik (Migrated from github.com) commented

2024-04-09 18:03:58 +00:00

These settings should come from the common, release and debug configs, like it's done in other configurations in this project.

iphone/Maps/Maps.xcodeproj/xcshareddata/xcschemes/OMaps.xcscheme

					
				@ -30,0 +39,4 @@

				      <CommandLineArguments>

				         <CommandLineArgument

				            argument = "-IsTests"

				            isEnabled = "YES">

biodranik (Migrated from github.com) commented

2024-04-09 18:05:59 +00:00

On the second thought, we already know if tests are launched either from XCode or from command line, even without that parameter, right?

iphone/Maps/Maps.xcodeproj/xcshareddata/xcschemes/OMaps.xcscheme

biodranik (Migrated from github.com) commented

2024-04-09 18:05:07 +00:00

Are env vars or command line params used in the final solution?

kirylkaveryn reviewed 2024-04-10 07:46:23 +00:00

iphone/Maps/Maps.xcodeproj/xcshareddata/xcschemes/OMaps.xcscheme

					
				@ -30,0 +39,4 @@

				      <CommandLineArguments>

				         <CommandLineArgument

				            argument = "-IsTests"

				            isEnabled = "YES">

kirylkaveryn commented

2024-04-10 07:46:23 +00:00

Yes! This launch arg is used only during the test run started by both xcode and console.

kirylkaveryn reviewed 2024-04-10 08:53:39 +00:00

iphone/Maps/Maps.xcodeproj/project.pbxproj

					
				@ -4598,1 +4664,4 @@

						};

						ED097E7C2BB80C330006ED01 /* Debug */ = {

							isa = XCBuildConfiguration;

							baseConfigurationReference = 347526FA1DC0B00F00918CF5 /* common-debug.xcconfig */;

kirylkaveryn commented

2024-04-10 08:53:38 +00:00

All settings are following the common.xcconfig. When I change this or other settings and save the file - they all will be automatically updated.
The fact that the property is set here with the concrete value - this is how xcconfig works:

All settings are following the `common.xcconfig`. When I change this or other settings and save the file - they all will be automatically updated. The fact that the property is set here with the concrete value - this is how xcconfig works: <img width="904" alt="image" src="https://github.com/organicmaps/organicmaps/assets/79797627/7226c09c-cde4-433c-b2d6-33a77ba67007">

biodranik (Migrated from github.com) reviewed 2024-04-10 16:29:56 +00:00

biodranik (Migrated from github.com) left a comment

I have code signing issues when running it. Does it work fine for you on the simulator and the device?

iphone/Maps/Maps.xcodeproj/project.pbxproj

biodranik (Migrated from github.com) commented

2024-04-10 16:21:30 +00:00

Is it the latest Xcode?

iphone/Maps/Maps.xcodeproj/project.pbxproj

					
				@ -4598,1 +4664,4 @@

						};

						ED097E7C2BB80C330006ED01 /* Debug */ = {

							isa = XCBuildConfiguration;

							baseConfigurationReference = 347526FA1DC0B00F00918CF5 /* common-debug.xcconfig */;

biodranik (Migrated from github.com) commented

2024-04-10 16:28:29 +00:00

app.organicmaps.tests

biodranik (Migrated from github.com) commented

2024-04-10 16:29:14 +00:00

app.organicmaps.tests

iphone/Maps/Maps.xcodeproj/project.pbxproj

biodranik (Migrated from github.com) commented

2024-04-10 16:28:56 +00:00

				TEST_HOST = "$(BUILT_PRODUCTS_DIR)/Organic Maps.app/$(BUNDLE_EXECUTABLE_FOLDER_PATH)/Organic Maps";

Did you test it in Release?

```suggestion TEST_HOST = "$(BUILT_PRODUCTS_DIR)/Organic Maps.app/$(BUNDLE_EXECUTABLE_FOLDER_PATH)/Organic Maps"; ``` Did you test it in Release?

kirylkaveryn reviewed 2024-04-10 17:34:28 +00:00

iphone/Maps/Maps.xcodeproj/project.pbxproj

kirylkaveryn commented

2024-04-10 17:34:28 +00:00

No. Latest is 15.3. Should this line be removed?

kirylkaveryn reviewed 2024-04-11 07:23:46 +00:00

iphone/Maps/Maps.xcodeproj/project.pbxproj

kirylkaveryn commented

2024-04-11 07:23:46 +00:00

Removed form .pbxproj

biodranik (Migrated from github.com) reviewed 2024-04-11 20:27:53 +00:00

biodranik (Migrated from github.com) left a comment

A few more comments. If it works on your device, let's move on with merging then.

It would be great to run tests also on Mac, without any simulator, we can do it later.

A few more comments. If it works on your device, let's move on with merging then. It would be great to run tests also on Mac, without any simulator, we can do it later.

iphone/Maps/Classes/MapViewController.mm

biodranik (Migrated from github.com) commented

2024-04-11 20:23:08 +00:00

  if (!self.mapView.drapeEngineCreated && !MapsAppDelegate.isDrapeDisabled)

```suggestion if (!self.mapView.drapeEngineCreated && !MapsAppDelegate.isDrapeDisabled) ```

iphone/Maps/Classes/MapsAppDelegate.h

biodranik (Migrated from github.com) commented

2024-04-11 20:22:49 +00:00

+ (BOOL)isDrapeDisabled;

```suggestion + (BOOL)isDrapeDisabled; ```

iphone/Maps/Classes/MapsAppDelegate.mm

biodranik (Migrated from github.com) commented

2024-04-11 20:22:57 +00:00

+ (BOOL)isDrapeDisabled {

```suggestion + (BOOL)isDrapeDisabled { ```

iphone/Maps/Maps.xcodeproj/project.pbxproj

biodranik (Migrated from github.com) commented

2024-04-11 20:23:59 +00:00

This line should be removed, it is duplicated below.

iphone/Maps/Maps.xcodeproj/project.pbxproj

					
				@ -4596,6 +4662,35 @@

							};

biodranik (Migrated from github.com) commented

2024-04-11 20:24:31 +00:00

				PRODUCT_BUNDLE_IDENTIFIER = app.organicmaps.debug.tests;

```suggestion PRODUCT_BUNDLE_IDENTIFIER = app.organicmaps.debug.tests; ```

biodranik (Migrated from github.com) commented

2024-04-11 20:25:39 +00:00

				PRODUCT_BUNDLE_IDENTIFIER = app.organicmaps.release.tests;

```suggestion PRODUCT_BUNDLE_IDENTIFIER = app.organicmaps.release.tests; ```

iphone/Maps/Maps.xcodeproj/xcshareddata/xcschemes/OMaps.xcscheme

					
				@ -30,0 +39,4 @@

				      <CommandLineArguments>

				         <CommandLineArgument

				            argument = "-IsTests"

				            isEnabled = "YES">

biodranik (Migrated from github.com) commented

2024-04-11 20:26:52 +00:00

Maybe make it more explicit, like -disableDrape ?

iphone/Maps/UI/CarPlay/CarPlayMapViewController.swift

biodranik (Migrated from github.com) commented

2024-04-11 20:27:09 +00:00

    if mapView?.drapeEngineCreated == false && !MapsAppDelegate.isDrapeDisabled() {

```suggestion if mapView?.drapeEngineCreated == false && !MapsAppDelegate.isDrapeDisabled() { ```

biodranik (Migrated from github.com) approved these changes 2024-04-12 11:21:36 +00:00

biodranik (Migrated from github.com) left a comment

Thanks! Let it 🪰

This repo is archived. You cannot comment on pull requests.

No reviewers

biodranik

No labels

Android Automotive (AAOS)

API

AppGallery

AppStore

Battery and Performance

External Map Datasets

F-Droid

Fonts

Frequently User Reported

No milestone

No project

No assignees

2 participants

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: organicmaps/organicmaps-tmp#7740

				`@ -4596,6 +4662,35 @@`
				`};`

Rows
Columns