Skip to content

Conversation

@islameldesoky95
Copy link
Collaborator

@islameldesoky95 islameldesoky95 commented Oct 8, 2025

Summary by CodeRabbit

  • Tests

    • Added a new iOS test target with extensive unit tests, fakes, mocks, mock factories, and helpers covering module API, configuration, readiness, session restore, consent/TCF/CCPA flows, serialization, banner display, analytics, language changes, and error paths.
  • Chores

    • Integrated the test target into the project and dependency setup with per-target build configs and test-specific build phases and references.
    • Added many test-only mock factory helpers.
  • CI

    • CI updated to macOS-14/Xcode 16.1 with Ruby/Bundler-based pod install, updated test device/runtime and test invocation.

@coderabbitai
Copy link

coderabbitai bot commented Oct 8, 2025

Walkthrough

Adds an iOS XCTest target "sampleTests" with many new test sources, fakes, and mock builders; updates the Xcode project to include the test target and related build/config entries; adjusts Podfile/Gemfile for test handling; and modifies CI to run on macOS-14 / Xcode 16.1 using Bundler-based pod install and a new test destination.

Changes

Cohort / File(s) Summary
Xcode project
sample/ios/sample.xcodeproj/project.pbxproj
Adds sampleTests PBXNativeTarget and related entries: PBXContainerItemProxy, PBXTargetDependency, PBXSources/PBXFrameworks/PBXResources build phases, PBXFileReference entries (e.g., sampleTests.xctest, Pods_sampleTests.framework, per-target xcconfigs), XCBuildConfiguration/XCConfigurationList for sampleTests, updates PBXProject attributes (TestTargetID, LastSwiftUpdateCheck, CreatedOnToolsVersion), and inserts sampleTests into project targets/groups.
Test target sources
sample/ios/sampleTests/*.swift
Adds many unit test files including RNUsercentricsModuleTests.swift, BannerSettingsDictTests.swift, FirstLayerOptionsDictTests.swift, ReadyStatusDictTests.swift, TCFDecisionUILayerTests.swift, UserDecisionDictTests.swift, UsercentricsAnalyticsEventTypeTests.swift, UsercentricsConsentTypeTests.swift, UsercentricsOptionsDictTests.swift, UsercentricsUserInteractionDictTest.swift, and additional test suites.
Test fakes
sample/ios/sampleTests/Fake/*
Adds FakeDispatchQueue (executes async blocks synchronously), FakePresentationViewController, and FakeUsercentricsManager — a configurable test double implementing UsercentricsManager APIs with controllable responses and state.
Mock builders
sample/ios/sampleTests/Mock/*
Adds numerous static mock() factory methods and serialization helpers for CMP/TCF data, TCF decisions, UserDecision, ReadyStatus, UsercentricsOptions, and related types to provide consistent test data and dictionary representations.
Podfile
sample/ios/Podfile
Adds target 'sampleTests' with inherit! :search_paths, updates react_native_pods.rb invocation line, and refactors post_install to iterate non-test targets and apply inherited C++ build flags (c++20/libc++) safely; adds comments about non-blocking pre_install and CI/macOS behavior.
Gemfile
sample/Gemfile
Bumps Ruby requirement to >= 3.2.0, updates gem constraints (e.g., pins cocoapods and xcodeproj to newer compatible ranges) and adds xcpretty.
CI / Tooling
.github/workflows/ci.yml
Updates iOS CI to macos-14 and Xcode 16.1; adds Ruby setup (ruby/setup-ruby@v1, Ruby 3.2) and Bundler steps (bundle install, bundle exec pod install --repo-update); adds clean step; and updates xcodebuild test destination to iOS 17.5 iPhone 15 with -only-testing:sampleTests.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Tester as XCTest
  participant RNModule as RNUsercentricsModule
  participant Queue as FakeDispatchQueue
  participant Manager as FakeUsercentricsManager

  Tester->>RNModule: invoke API (e.g., isReady / configure / showFirstLayer)
  RNModule->>Queue: dispatch async { call Manager.* }
  Note right of Queue: FakeDispatchQueue executes immediately (synchronous)
  Queue-->>RNModule: yields to RNModule
  RNModule->>Manager: perform operation with callbacks
  Manager-->>RNModule: success or failure (preconfigured)
  RNModule-->>Tester: resolves promise / calls callback with dict or error
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • uc-brunosilva

Poem

I nibble code and hop through tests,
New targets sprout in tidy nests.
Fake queues race and mocks reply,
Enums and dicts learn where to fly.
Carrots for green checks in the sky! 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly describes the main change of adding a test folder to the sample project and updating tests, which aligns with the extensive test target and file additions in this changeset.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/MSDK-2613-fix-ios-sample-tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (13)
sample/ios/sampleTests/UsercentricsConsentTypeTests.swift (2)

8-11: Consider adding edge case tests.

The test only covers the happy path with valid inputs (0 and 1). Consider adding tests for:

  • Invalid integer values (e.g., -1, 2, or larger values)
  • Boundary conditions if applicable
  • Default/fallback behavior

Example:

func testInitializeFromInt() {
  XCTAssertEqual(.explicit_, UsercentricsConsentType.initialize(from: 0))
  XCTAssertEqual(.implicit, UsercentricsConsentType.initialize(from: 1))
}

func testInitializeFromInvalidInt() {
  // Add assertions for invalid input handling
  // Example: XCTAssertNil(UsercentricsConsentType.initialize(from: -1))
  // or verify it returns a default value
}

8-11: Rename test methods to reflect initialize(from:) behavior

  • In sample/ios/sampleTests/UsercentricsConsentTypeTests.swift (line 8), rename
    func testToIntMethod()func testInitializeFromInt()
  • In sample/ios/sampleTests/TCFDecisionUILayerTests.swift (line 7), rename
    func testToIntMethod()func testInitializeFromInt()
sample/ios/sampleTests/Mock/UsercentricsOptions+Mock.swift (1)

17-18: Simplify the force unwrap.

Line 18 checks for nil then immediately force unwraps, which is safe but awkward. Consider simplifying:

Apply this diff:

-  static func asDict(mockObject: UsercentricsOptions? = nil) -> NSDictionary {
-    let expectedOptions = (mockObject == nil ? self.mock() : mockObject)!
+  static func asDict(mockObject: UsercentricsOptions? = nil) -> NSDictionary {
+    let expectedOptions = mockObject ?? self.mock()
sample/ios/sampleTests/UsercentricsAnalyticsEventTypeTests.swift (1)

6-11: Consider testing additional enum values for completeness.

The test verifies one mapping (1 → .acceptAllFirstLayer), but testing other enum values would improve coverage and catch potential initialization errors.

sample/ios/sampleTests/FirstLayerOptionsDictTests.swift (2)

46-46: Reminder: Address the TODO comment.

The TODO indicates that ButtonSettings properties need to be exposed for proper test validation. Currently, the test only verifies non-nil without checking actual values.

Do you want me to help generate the assertions once ButtonSettings properties are exposed, or open a new issue to track this task?


66-66: Consider adding descriptions to XCTFail calls.

XCTFail calls without descriptions can make test failures harder to debug. Consider adding descriptive messages to clarify what conditions should have been met.

For example, on Line 66:

-        XCTFail()
+        XCTFail("Expected ButtonLayout to be .row but got a different layout type")

Apply similar descriptive messages to the XCTFail calls on lines 79, 93, 205, 217, and 229.

Also applies to: 79-79, 93-93, 205-205, 217-217, 229-229

sample/ios/sampleTests/ReadyStatusDictTests.swift (1)

25-34: Consider using safer casting patterns in tests.

The tests use force casts (as!) extensively throughout, which can crash at runtime if the serialization produces unexpected types. While force casts in tests can provide fail-fast behavior, they make debugging harder when types don't match expectations.

Consider using optional binding with XCTAssertNotNil or guard statements for safer, more debuggable tests. For example, on lines 25-26:

-    XCTAssertEqual(false, dictionary["shouldCollectConsent"] as! Bool)
-    XCTAssertNotNil(dictionary["consents"] as? [NSDictionary])
+    guard let shouldCollectConsent = dictionary["shouldCollectConsent"] as? Bool else {
+      XCTFail("shouldCollectConsent should be a Bool")
+      return
+    }
+    XCTAssertEqual(false, shouldCollectConsent)
+    XCTAssertNotNil(dictionary["consents"] as? [NSDictionary])

This pattern provides clearer error messages when types don't match and makes the tests more maintainable.

Also applies to: 48-53, 65-65, 71-73

sample/ios/sampleTests/Mock/CMPData+Mock.swift (1)

109-112: Avoid force-cast in tests (SwiftLint error).

The dynamic allocation with NSClassFromString uses a force-cast and triggers SwiftLint. If replacing with a safe cast/guard is awkward here, locally disable the rule in this small scope.

Apply this minimal change:

   static func mock() -> LegalBasisLocalization {
-    let value = (NSClassFromString("UsercentricsLegalBasisLocalization")?.alloc() as! LegalBasisLocalization)
+    // swiftlint:disable force_cast
+    let value = (NSClassFromString("UsercentricsLegalBasisLocalization")?.alloc() as! LegalBasisLocalization)
+    // swiftlint:enable force_cast
     return value.doCopy(labels: 123, labelsAria: .mock(), data: ["123": "123"])
   }
sample/ios/sampleTests/RNUsercentricsModuleTests.swift (2)

201-206: Replace unused closure parameters with _ (SwiftLint warning).

The success closures don’t use their parameters. Rename them to _ to silence warnings.

-  module.changeLanguage("pt") { result in
+  module.changeLanguage("pt") { _ in
@@
-  module.showFirstLayer(emptyDictionary) { result in
+  module.showFirstLayer(emptyDictionary) { _ in
@@
-  module.showSecondLayer(serializedSettings) { result in
+  module.showSecondLayer(serializedSettings) { _ in
@@
-  module.showSecondLayer(emptyDictionary) { result in
+  module.showSecondLayer(emptyDictionary) { _ in

Also applies to: 493-499, 505-510, 515-520


56-58: Add XCTFail messages (SwiftLint warning).

Bare XCTFail calls reduce diagnosability. Add a short reason per site, e.g., XCTFail("result should not be nil") or XCTFail("Should not reach reject").

Also applies to: 98-100, 150-152, 170-172, 186-188, 226-228, 259-261, 295-297, 328-330, 365-367, 386-388, 408-410, 428-430, 534-536, 575-577, 602-604

sample/ios/sampleTests/Fake/FakeUsercentricsManager.swift (3)

108-110: Remove redundant optional initializations.

Explicit = nil on optionals is redundant.

-  var changeLanguageValue: String? = nil
-  var changeLanguageError: Error? = nil
+  var changeLanguageValue: String?
+  var changeLanguageError: Error?

68-70: Avoid force unwrap when returning an optional.

The function returns String?; return the backing optional directly.

-  func getABTestingVariant() -> String? {
-    return getABTestingVariantValue!
-  }
+  func getABTestingVariant() -> String? {
+    return getABTestingVariantValue
+  }

190-193: Prefer shorthand array syntax.

Use [UsercentricsAnalyticsEventType] instead of Array<UsercentricsAnalyticsEventType>.

-  var trackCalls: Array<UsercentricsAnalyticsEventType> = []
+  var trackCalls: [UsercentricsAnalyticsEventType] = []
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ae9bbb7 and 74a9599.

📒 Files selected for processing (20)
  • sample/ios/sample.xcodeproj/project.pbxproj (9 hunks)
  • sample/ios/sampleTests/BannerSettingsDictTests.swift (1 hunks)
  • sample/ios/sampleTests/Fake/FakeDispatchQueue.swift (1 hunks)
  • sample/ios/sampleTests/Fake/FakePresentationViewController.swift (1 hunks)
  • sample/ios/sampleTests/Fake/FakeUsercentricsManager.swift (1 hunks)
  • sample/ios/sampleTests/FirstLayerOptionsDictTests.swift (1 hunks)
  • sample/ios/sampleTests/Mock/CMPData+Mock.swift (1 hunks)
  • sample/ios/sampleTests/Mock/TCFData+Mock.swift (1 hunks)
  • sample/ios/sampleTests/Mock/TCFUserDecisions+Mock.swift (1 hunks)
  • sample/ios/sampleTests/Mock/UserDecisions+Mock.swift (1 hunks)
  • sample/ios/sampleTests/Mock/UsercentricsIsReady+Mock.swift (1 hunks)
  • sample/ios/sampleTests/Mock/UsercentricsOptions+Mock.swift (1 hunks)
  • sample/ios/sampleTests/RNUsercentricsModuleTests.swift (1 hunks)
  • sample/ios/sampleTests/ReadyStatusDictTests.swift (1 hunks)
  • sample/ios/sampleTests/TCFDecisionUILayerTests.swift (1 hunks)
  • sample/ios/sampleTests/UserDecisionDictTests.swift (1 hunks)
  • sample/ios/sampleTests/UsercentricsAnalyticsEventTypeTests.swift (1 hunks)
  • sample/ios/sampleTests/UsercentricsConsentTypeTests.swift (1 hunks)
  • sample/ios/sampleTests/UsercentricsOptionsDictTests.swift (1 hunks)
  • sample/ios/sampleTests/UsercentricsUserInteractionDictTest.swift (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (16)
sample/ios/sampleTests/Mock/UsercentricsIsReady+Mock.swift (4)
src/models/UsercentricsReadyStatus.tsx (1)
  • UsercentricsReadyStatus (4-20)
src/models/GeolocationRuleset.tsx (1)
  • GeolocationRuleset (1-10)
src/models/UsercentricsLocation.tsx (1)
  • UsercentricsLocation (1-32)
src/models/UsercentricsServiceConsent.tsx (2)
  • UsercentricsServiceConsent (3-24)
  • UsercentricsConsentHistoryEntry (26-37)
sample/ios/sampleTests/TCFDecisionUILayerTests.swift (3)
src/Usercentrics.tsx (1)
  • Usercentrics (23-145)
sample/ios/sampleTests/UsercentricsConsentTypeTests.swift (1)
  • testToIntMethod (8-11)
sample/ios/sampleTests/UsercentricsUserInteractionDictTest.swift (1)
  • testToIntMethod (9-14)
sample/ios/sampleTests/UsercentricsOptionsDictTests.swift (1)
src/models/UsercentricsOptions.tsx (1)
  • UsercentricsOptions (3-45)
sample/ios/sampleTests/Mock/TCFUserDecisions+Mock.swift (2)
src/models/TCFUserDecisions.tsx (1)
  • TCFUserDecisions (1-24)
sample/ios/sampleTests/Mock/UserDecisions+Mock.swift (1)
  • mockToDict (5-11)
sample/ios/sampleTests/Mock/UserDecisions+Mock.swift (2)
sample/ios/sampleTests/Mock/TCFUserDecisions+Mock.swift (1)
  • mockToDict (12-20)
src/models/UserDecision.tsx (1)
  • UserDecision (1-10)
sample/ios/sampleTests/UsercentricsUserInteractionDictTest.swift (1)
ios/Extensions/UsercentricsUserInteraction+String.swift (1)
  • toInt (5-17)
sample/ios/sampleTests/Mock/TCFData+Mock.swift (2)
src/Usercentrics.tsx (1)
  • Usercentrics (23-145)
src/models/TCFData.tsx (2)
  • TCFVendorRestriction (279-291)
  • VendorUrl (262-277)
sample/ios/sampleTests/FirstLayerOptionsDictTests.swift (1)
src/models/FirstLayerSettings.tsx (11)
  • ButtonSettings (135-162)
  • ButtonLayout (106-127)
  • row (116-118)
  • column (120-122)
  • grid (124-126)
  • MessageSettings (82-104)
  • TitleSettings (67-80)
  • HeaderImageSettings (34-65)
  • extended (54-56)
  • hidden (62-64)
  • logo (58-60)
sample/ios/sampleTests/UsercentricsAnalyticsEventTypeTests.swift (1)
src/Usercentrics.tsx (1)
  • Usercentrics (23-145)
sample/ios/sampleTests/Mock/UsercentricsOptions+Mock.swift (1)
src/models/UsercentricsOptions.tsx (1)
  • UsercentricsOptions (3-45)
sample/ios/sampleTests/ReadyStatusDictTests.swift (5)
src/models/UsercentricsServiceConsent.tsx (2)
  • UsercentricsConsentHistoryEntry (26-37)
  • UsercentricsServiceConsent (3-24)
src/models/UsercentricsReadyStatus.tsx (1)
  • UsercentricsReadyStatus (4-20)
src/models/GeolocationRuleset.tsx (1)
  • GeolocationRuleset (1-10)
src/models/UsercentricsLocation.tsx (1)
  • UsercentricsLocation (1-32)
sample/ios/sampleTests/Mock/TCFUserDecisions+Mock.swift (4)
  • toDictionary (30-36)
  • toDictionary (45-50)
  • toDictionary (60-66)
  • toDictionary (74-79)
sample/ios/sampleTests/BannerSettingsDictTests.swift (2)
sample/ios/sampleTests/UserDecisionDictTests.swift (1)
  • testFromDictWithCorrectValues (8-15)
src/models/BannerSettings.tsx (1)
  • BannerSettings (104-117)
sample/ios/sampleTests/Mock/CMPData+Mock.swift (5)
sample/ios/sampleTests/Mock/TCFData+Mock.swift (7)
  • mock (5-14)
  • mock (18-23)
  • mock (27-39)
  • mock (43-52)
  • mock (56-62)
  • mock (66-71)
  • mock (75-102)
sample/ios/sampleTests/Mock/TCFUserDecisions+Mock.swift (5)
  • mock (5-10)
  • mock (24-28)
  • mock (40-43)
  • mock (54-58)
  • mock (70-72)
sample/ios/sampleTests/Mock/UsercentricsIsReady+Mock.swift (3)
  • mock (12-17)
  • mock (21-30)
  • mock (34-36)
sample/ios/sampleTests/Mock/UsercentricsOptions+Mock.swift (1)
  • mock (5-15)
src/models/UsercentricsLocation.tsx (1)
  • UsercentricsLocation (1-32)
sample/ios/sampleTests/RNUsercentricsModuleTests.swift (4)
ios/RNUsercentricsModule.swift (1)
  • moduleName (23-25)
sample/ios/sampleTests/Mock/CMPData+Mock.swift (16)
  • mock (5-11)
  • mock (16-51)
  • mock (56-104)
  • mock (109-112)
  • mock (117-217)
  • mock (222-231)
  • mock (236-238)
  • mock (243-252)
  • mock (258-276)
  • mock (281-343)
  • mock (348-356)
  • mock (361-367)
  • mock (372-398)
  • mock (403-407)
  • mock (412-415)
  • mock (420-432)
sample/ios/sampleTests/Fake/FakeUsercentricsManager.swift (25)
  • configure (43-46)
  • isReady (33-41)
  • restoreUserSession (48-58)
  • getControllerId (64-66)
  • getABTestingVariant (68-70)
  • getConsents (73-75)
  • getUserSessionData (83-85)
  • getUSPData (88-90)
  • changeLanguage (110-117)
  • acceptAll (130-133)
  • acceptAllForTCF (122-126)
  • denyAll (146-149)
  • denyAllForTCF (138-142)
  • saveDecisionsForTCF (152-154)
  • saveDecisions (157-159)
  • saveOptOutForCCPA (162-164)
  • getTCFData (93-95)
  • setCMPId (167-169)
  • setABTestingVariant (172-174)
  • showFirstLayer (178-182)
  • showSecondLayer (185-188)
  • track (191-193)
  • getCMPData (78-80)
  • getAdditionalConsentModeData (98-100)
  • clearUserSession (195-204)
sample/ios/sampleTests/Mock/UsercentricsOptions+Mock.swift (1)
  • asDict (17-29)
sample/ios/sampleTests/UsercentricsConsentTypeTests.swift (3)
src/Usercentrics.tsx (1)
  • Usercentrics (23-145)
sample/ios/sampleTests/TCFDecisionUILayerTests.swift (1)
  • testToIntMethod (8-11)
sample/ios/sampleTests/UsercentricsUserInteractionDictTest.swift (1)
  • testToIntMethod (9-14)
sample/ios/sampleTests/Fake/FakeUsercentricsManager.swift (2)
src/Usercentrics.tsx (1)
  • Usercentrics (23-145)
src/models/UsercentricsConsentUserResponse.tsx (1)
  • UsercentricsConsentUserResponse (3-14)
🪛 SwiftLint (0.57.0)
sample/ios/sampleTests/FirstLayerOptionsDictTests.swift

[Warning] 46-46: TODOs should be resolved (Expose ButtonSettings properti...)

(todo)


[Warning] 66-66: An XCTFail call should include a description of the assertion

(xctfail_message)


[Warning] 79-79: An XCTFail call should include a description of the assertion

(xctfail_message)


[Warning] 93-93: An XCTFail call should include a description of the assertion

(xctfail_message)


[Warning] 205-205: An XCTFail call should include a description of the assertion

(xctfail_message)


[Warning] 217-217: An XCTFail call should include a description of the assertion

(xctfail_message)


[Warning] 229-229: An XCTFail call should include a description of the assertion

(xctfail_message)

sample/ios/sampleTests/ReadyStatusDictTests.swift

[Error] 25-25: Force casts should be avoided

(force_cast)


[Error] 28-28: Force casts should be avoided

(force_cast)


[Error] 29-29: Force casts should be avoided

(force_cast)


[Error] 30-30: Force casts should be avoided

(force_cast)


[Error] 32-32: Force casts should be avoided

(force_cast)


[Error] 33-33: Force casts should be avoided

(force_cast)


[Error] 34-34: Force casts should be avoided

(force_cast)


[Error] 48-48: Force casts should be avoided

(force_cast)


[Error] 49-49: Force casts should be avoided

(force_cast)


[Error] 50-50: Force casts should be avoided

(force_cast)


[Error] 51-51: Force casts should be avoided

(force_cast)


[Error] 52-52: Force casts should be avoided

(force_cast)


[Error] 53-53: Force casts should be avoided

(force_cast)


[Error] 65-65: Force casts should be avoided

(force_cast)


[Error] 71-71: Force casts should be avoided

(force_cast)


[Error] 72-72: Force casts should be avoided

(force_cast)


[Error] 73-73: Force casts should be avoided

(force_cast)

sample/ios/sampleTests/Mock/CMPData+Mock.swift

[Error] 110-110: Force casts should be avoided

(force_cast)

sample/ios/sampleTests/RNUsercentricsModuleTests.swift

[Error] 62-62: Force casts should be avoided

(force_cast)


[Error] 63-63: Force casts should be avoided

(force_cast)


[Error] 65-65: Force casts should be avoided

(force_cast)


[Error] 66-66: Force casts should be avoided

(force_cast)


[Error] 68-68: Force casts should be avoided

(force_cast)


[Error] 69-69: Force casts should be avoided

(force_cast)


[Error] 70-70: Force casts should be avoided

(force_cast)


[Error] 71-71: Force casts should be avoided

(force_cast)


[Error] 72-72: Force casts should be avoided

(force_cast)


[Error] 103-103: Force casts should be avoided

(force_cast)


[Error] 104-104: Force casts should be avoided

(force_cast)


[Error] 105-105: Force casts should be avoided

(force_cast)


[Error] 106-106: Force casts should be avoided

(force_cast)


[Error] 107-107: Force casts should be avoided

(force_cast)


[Error] 154-154: Force casts should be avoided

(force_cast)


[Error] 155-155: Force casts should be avoided

(force_cast)


[Error] 156-156: Force casts should be avoided

(force_cast)


[Error] 157-157: Force casts should be avoided

(force_cast)


[Error] 158-158: Force casts should be avoided

(force_cast)


[Error] 190-190: Force casts should be avoided

(force_cast)


[Error] 191-191: Force casts should be avoided

(force_cast)


[Error] 192-192: Force casts should be avoided

(force_cast)


[Error] 193-193: Force casts should be avoided

(force_cast)


[Error] 194-194: Force casts should be avoided

(force_cast)


[Error] 230-230: Force casts should be avoided

(force_cast)


[Error] 231-231: Force casts should be avoided

(force_cast)


[Error] 232-232: Force casts should be avoided

(force_cast)


[Error] 233-233: Force casts should be avoided

(force_cast)


[Error] 234-234: Force casts should be avoided

(force_cast)


[Error] 263-263: Force casts should be avoided

(force_cast)


[Error] 264-264: Force casts should be avoided

(force_cast)


[Error] 265-265: Force casts should be avoided

(force_cast)


[Error] 266-266: Force casts should be avoided

(force_cast)


[Error] 267-267: Force casts should be avoided

(force_cast)


[Error] 299-299: Force casts should be avoided

(force_cast)


[Error] 300-300: Force casts should be avoided

(force_cast)


[Error] 301-301: Force casts should be avoided

(force_cast)


[Error] 302-302: Force casts should be avoided

(force_cast)


[Error] 303-303: Force casts should be avoided

(force_cast)


[Error] 332-332: Force casts should be avoided

(force_cast)


[Error] 333-333: Force casts should be avoided

(force_cast)


[Error] 334-334: Force casts should be avoided

(force_cast)


[Error] 335-335: Force casts should be avoided

(force_cast)


[Error] 336-336: Force casts should be avoided

(force_cast)


[Error] 369-369: Force casts should be avoided

(force_cast)


[Error] 370-370: Force casts should be avoided

(force_cast)


[Error] 371-371: Force casts should be avoided

(force_cast)


[Error] 372-372: Force casts should be avoided

(force_cast)


[Error] 373-373: Force casts should be avoided

(force_cast)


[Error] 390-390: Force casts should be avoided

(force_cast)


[Error] 391-391: Force casts should be avoided

(force_cast)


[Error] 392-392: Force casts should be avoided

(force_cast)


[Error] 393-393: Force casts should be avoided

(force_cast)


[Error] 394-394: Force casts should be avoided

(force_cast)


[Error] 412-412: Force casts should be avoided

(force_cast)


[Error] 413-413: Force casts should be avoided

(force_cast)


[Error] 414-414: Force casts should be avoided

(force_cast)


[Error] 415-415: Force casts should be avoided

(force_cast)


[Error] 416-416: Force casts should be avoided

(force_cast)


[Error] 579-579: Force casts should be avoided

(force_cast)


[Error] 584-584: Force casts should be avoided

(force_cast)


[Error] 585-585: Force casts should be avoided

(force_cast)


[Error] 586-586: Force casts should be avoided

(force_cast)


[Error] 587-587: Force casts should be avoided

(force_cast)


[Error] 607-607: Force casts should be avoided

(force_cast)


[Error] 608-608: Force casts should be avoided

(force_cast)


[Error] 609-609: Force casts should be avoided

(force_cast)


[Error] 610-610: Force casts should be avoided

(force_cast)


[Error] 611-611: Force casts should be avoided

(force_cast)


[Warning] 210-210: Unused parameter in a closure should be replaced with _

(unused_closure_parameter)


[Warning] 485-485: Unused parameter in a closure should be replaced with _

(unused_closure_parameter)


[Warning] 494-494: Unused parameter in a closure should be replaced with _

(unused_closure_parameter)


[Warning] 505-505: Unused parameter in a closure should be replaced with _

(unused_closure_parameter)


[Warning] 515-515: Unused parameter in a closure should be replaced with _

(unused_closure_parameter)


[Warning] 56-56: An XCTFail call should include a description of the assertion

(xctfail_message)


[Warning] 98-98: An XCTFail call should include a description of the assertion

(xctfail_message)


[Warning] 150-150: An XCTFail call should include a description of the assertion

(xctfail_message)


[Warning] 170-170: An XCTFail call should include a description of the assertion

(xctfail_message)


[Warning] 186-186: An XCTFail call should include a description of the assertion

(xctfail_message)


[Warning] 226-226: An XCTFail call should include a description of the assertion

(xctfail_message)


[Warning] 259-259: An XCTFail call should include a description of the assertion

(xctfail_message)


[Warning] 295-295: An XCTFail call should include a description of the assertion

(xctfail_message)


[Warning] 328-328: An XCTFail call should include a description of the assertion

(xctfail_message)


[Warning] 365-365: An XCTFail call should include a description of the assertion

(xctfail_message)


[Warning] 386-386: An XCTFail call should include a description of the assertion

(xctfail_message)


[Warning] 408-408: An XCTFail call should include a description of the assertion

(xctfail_message)


[Warning] 428-428: An XCTFail call should include a description of the assertion

(xctfail_message)


[Warning] 534-534: An XCTFail call should include a description of the assertion

(xctfail_message)


[Warning] 575-575: An XCTFail call should include a description of the assertion

(xctfail_message)


[Warning] 602-602: An XCTFail call should include a description of the assertion

(xctfail_message)

sample/ios/sampleTests/Fake/FakeUsercentricsManager.swift

[Warning] 108-108: Initializing an optional variable with nil is redundant

(redundant_optional_initialization)


[Warning] 109-109: Initializing an optional variable with nil is redundant

(redundant_optional_initialization)


[Warning] 190-190: Shorthand syntactic sugar should be used, i.e. [Int] instead of Array

(syntactic_sugar)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test-ios
  • GitHub Check: test-android
🔇 Additional comments (13)
sample/ios/sampleTests/UsercentricsConsentTypeTests.swift (1)

1-5: LGTM! Standard test setup.

The imports are correctly configured with @testable for accessing internal members during testing.

sample/ios/sampleTests/Fake/FakePresentationViewController.swift (1)

1-4: LGTM!

The fake presentation controller implementation is appropriate for test purposes. The empty subclass serves as a stub to satisfy presentation requirements in unit tests.

sample/ios/sample.xcodeproj/project.pbxproj (1)

17-25: LGTM! Test target properly configured.

The Xcode project changes correctly add the sampleTests target with appropriate:

  • Container item proxy for target dependency
  • File system synchronized root group for test files
  • Native target with build phases (Check Pods Manifest.lock, Sources, Frameworks, Resources)
  • Product reference and group membership
  • Target dependency linking sample and sampleTests

Note: Two separate "Check Pods Manifest.lock" phases exist (lines 235 and 291) - one for each target. This is correct and expected.

Also applies to: 40-52, 99-172, 200-200, 274-312, 324-339

sample/ios/sampleTests/TCFDecisionUILayerTests.swift (1)

6-12: LGTM!

The test correctly verifies that TCFDecisionUILayer.initialize(from:) maps integer values to the appropriate enum cases.

sample/ios/sampleTests/UsercentricsUserInteractionDictTest.swift (1)

7-15: LGTM!

The test correctly verifies the toInt() mapping for all UsercentricsUserInteraction enum cases, matching the production implementation.

sample/ios/sampleTests/Mock/UserDecisions+Mock.swift (1)

4-12: LGTM!

The mock factory method correctly creates a UserDecision instance and serializes it to a dictionary format that matches the model structure.

sample/ios/sampleTests/UserDecisionDictTests.swift (1)

6-23: LGTM!

The tests provide good coverage for UserDecision dictionary initialization, covering both successful initialization with correct keys and failure cases with incorrect keys.

sample/ios/sampleTests/Fake/FakeDispatchQueue.swift (1)

1-9: LGTM!

The fake implementation correctly executes async work synchronously, which is appropriate for deterministic testing. This test double will help isolate the code under test from real dispatch queue behavior.

sample/ios/sampleTests/Mock/UsercentricsIsReady+Mock.swift (1)

11-37: LGTM!

The mock builders provide consistent test data with representative values. This approach enables predictable test scenarios across the test suite.

sample/ios/sampleTests/UsercentricsOptionsDictTests.swift (1)

8-87: LGTM!

The test suite provides comprehensive coverage of UsercentricsOptions initialization from dictionaries, including edge cases like missing settings IDs and various enum value mappings. The tests are well-structured and should catch serialization issues effectively.

sample/ios/sampleTests/Mock/TCFData+Mock.swift (1)

4-103: LGTM!

The mock builders provide comprehensive test data for TCF-related models. The nested structure with cascading .mock() calls creates realistic test scenarios while keeping the code maintainable.

sample/ios/sampleTests/Mock/TCFUserDecisions+Mock.swift (1)

4-80: LGTM!

The mock builders and serialization helpers provide consistent test data for TCF decision models. The toDictionary() methods enable validation of the serialization logic, which is crucial for ensuring correct data exchange between native and JavaScript layers.

sample/ios/sampleTests/BannerSettingsDictTests.swift (1)

17-21: Ensure Swift BannerSettings exists and exposes required properties.

  • Confirm that the iOS module defines a BannerSettings type with generalStyleSettings?.logo, font?.fontName, and font?.pointSize.
  • Update tests: uncomment and fix these assertions if the properties are accessible; otherwise add a TODO explaining why they remain disabled or remove them entirely.

return [
"settingsId": expectedOptions.settingsId,
"ruleSetId": expectedOptions.ruleSetId,
"loggerLevel": expectedOptions.loggerLevel.ordinal,
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify .ordinal property exists on UsercentricsLoggerLevel.

Ensure the UsercentricsLoggerLevel enum has an .ordinal property. If this property doesn't exist, the code will fail at runtime.

Run the following script to verify the property exists:


🏁 Script executed:

#!/bin/bash
# Search for ordinal property definition on UsercentricsLoggerLevel
rg -nP --type=swift -C3 '\bUsercentricsLoggerLevel\b.*\bordinal\b'

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Inspect the definition and any extensions of UsercentricsLoggerLevel
rg -nP --type=swift 'enum\s+UsercentricsLoggerLevel\b' -C5
rg -nP --type=swift 'extension\s+UsercentricsLoggerLevel\b' -C5

Length of output: 779


UsercentricsLoggerLevel lacks .ordinal
The UsercentricsLoggerLevel enum does not define an ordinal property—use rawValue or add a computed var ordinal: Int { rawValue } to avoid runtime failures.

🤖 Prompt for AI Agents
In sample/ios/sampleTests/Mock/UsercentricsOptions+Mock.swift around line 22,
the test references UsercentricsLoggerLevel.ordinal which does not exist;
replace uses of .ordinal with .rawValue or add a computed property var ordinal:
Int { rawValue } to the enum. Update the mock to reference loggerLevel.rawValue
(or add the computed ordinal on the enum) so the serialized "loggerLevel" uses a
valid integer and prevents runtime failures.

Comment on lines +62 to +72
XCTAssertEqual("settingsId", geolocationRulesetMap["activeSettingsId"] as! String)
XCTAssertEqual(true, geolocationRulesetMap["bannerRequiredAtLocation"] as! Bool)

XCTAssertEqual("PT", locationMap["countryCode"] as! String)
XCTAssertEqual("PT11", locationMap["regionCode"] as! String)

XCTAssertEqual("1.2.3", consent["version"] as! String)
XCTAssertEqual("BBBB", consent["dataProcessor"] as! String)
XCTAssertEqual("AAAA", consent["templateId"] as! String)
XCTAssertEqual(0, consent["type"] as! Int)
XCTAssertEqual(true, consent["status"] as! Bool)
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

SwiftLint force_cast violations across tests — choose fix or disable for this file.

There are multiple as! casts flagged as errors. Options:

  • Preferred: replace with guarded typed extraction or a small helper to unwrap and assert type.
  • Pragmatic (tests-only): disable force_cast for this file.

Minimal file-level suppression:

+// swiftlint:disable force_cast
 import XCTest
 import UsercentricsUI
@@
 }
+
+// swiftlint:enable force_cast

Alternative: add a helper to avoid casts (example to place inside the test class):

private func expect<T>(_ dict: NSDictionary, _ key: String, as type: T.Type = T.self, file: StaticString = #filePath, line: UInt = #line) -> T {
  guard let value = dict[key] as? T else {
    XCTFail("Expected key '\(key)' as \(T.self)", file: file, line: line); fatalError()
  }
  return value
}

Then use:

let version: String = expect(consent, "version")
XCTAssertEqual("1.2.3", version)

Also applies to: 103-107, 154-158, 190-194, 230-234, 263-267, 299-303, 332-336, 369-373, 390-394, 412-416, 579-587, 607-611

🧰 Tools
🪛 SwiftLint (0.57.0)

[Error] 62-62: Force casts should be avoided

(force_cast)


[Error] 63-63: Force casts should be avoided

(force_cast)


[Error] 65-65: Force casts should be avoided

(force_cast)


[Error] 66-66: Force casts should be avoided

(force_cast)


[Error] 68-68: Force casts should be avoided

(force_cast)


[Error] 69-69: Force casts should be avoided

(force_cast)


[Error] 70-70: Force casts should be avoided

(force_cast)


[Error] 71-71: Force casts should be avoided

(force_cast)


[Error] 72-72: Force casts should be avoided

(force_cast)

🤖 Prompt for AI Agents
In sample/ios/sampleTests/RNUsercentricsModuleTests.swift around lines 62-72,
there are multiple forced casts using `as!` which trigger SwiftLint `force_cast`
violations; replace them with safe typed extraction by adding a small helper
method in the test class that attempts `as?` for the requested type, calls
`XCTFail` and terminates on mismatch, and then use that helper to retrieve
strongly-typed values before asserting, or alternatively (tests-only) add a
file-level SwiftLint disable for `force_cast` if you prefer the pragmatic
approach; apply the same pattern to the other flagged ranges (103-107, 154-158,
190-194, 230-234, 263-267, 299-303, 332-336, 369-373, 390-394, 412-416, 579-587,
607-611).

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 74a9599 and 878deed.

📒 Files selected for processing (1)
  • .github/workflows/ci.yml (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test-android
  • GitHub Check: test-ios
🔇 Additional comments (2)
.github/workflows/ci.yml (2)

68-74: Good call on purging stale iOS artifacts

Clearing Pods, build products, and DerivedData before installing ensures the new sample test target starts from a clean slate on CI. 👍


77-77: Thanks for forcing a repo update with pod install

pod install --repo-update keeps the CocoaPods specs fresh, which helps the new sampleTests target resolve dependencies reliably on the runner.

@uc-brunosouza uc-brunosouza force-pushed the chore/MSDK-2613-fix-ios-sample-tests branch 2 times, most recently from 414b2d6 to 596314a Compare October 8, 2025 15:36
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 414b2d6 and 596314a.

📒 Files selected for processing (1)
  • .github/workflows/ci.yml (4 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/ci.yml

23-23: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)


76-76: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)


124-124: missing input "distribution" which is required by action "actions/setup-java@v4". all required inputs are "distribution"

(action)


128-128: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: test-ios

Comment on lines 23 to 28
- uses: actions/cache@v3
with:
path: |
node_modules
key: ${{ runner.os }}-modules-${{ hashFiles('yarn.lock') }}

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Update actions/cache to v4 to avoid runtime failures.
actions/cache@v3 still runs on the deprecated Node 16 runtime; GitHub has started blocking those executions, so each cache step will exit before the job progresses. Bump every cache usage here to actions/cache@v4 (same inputs supported) to keep the workflow green.

Also applies to: 76-82, 128-133

🧰 Tools
🪛 actionlint (1.7.7)

23-23: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

🤖 Prompt for AI Agents
.github/workflows/ci.yml lines 23-28 (and also update occurrences at 76-82 and
128-133): the workflow is using actions/cache@v3 which runs on deprecated Node
16 and may be blocked; replace every actions/cache@v3 with actions/cache@v4 in
these blocks, keeping the existing with: inputs (path, key, restore-keys, etc.)
unchanged so behavior remains the same.

Comment on lines 124 to 127
uses: actions/setup-java@v4
with:
java-version: 17

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

actions/setup-java@v4 now requires distribution.
The v4 action fails fast unless you pass an explicit distribution (e.g. temurin, zulu). Add distribution: temurin (or your preferred build) alongside java-version so the Android job reaches Gradle.

🧰 Tools
🪛 actionlint (1.7.7)

124-124: missing input "distribution" which is required by action "actions/setup-java@v4". all required inputs are "distribution"

(action)

🤖 Prompt for AI Agents
.file: .github/workflows/ci.yml lines 124-127 — The GitHub Actions step using
actions/setup-java@v4 is missing the required distribution field; update that
step to include an explicit distribution (for example distribution: temurin)
alongside java-version so the action no longer fails fast and the Android job
can proceed to Gradle.

@islameldesoky95 islameldesoky95 force-pushed the chore/MSDK-2613-fix-ios-sample-tests branch 2 times, most recently from e21b1f5 to 19d303a Compare October 8, 2025 16:49
@uc-brunosouza uc-brunosouza force-pushed the chore/MSDK-2613-fix-ios-sample-tests branch from cee478b to 4834c9c Compare October 8, 2025 19:37
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
.github/workflows/ci.yml (1)

70-77: Update cache action to v4.

Line 70 still uses actions/cache@v3, which now runs on the deprecated Node 16 runtime and is blocked by GitHub-hosted runners, so this step will fail. Please bump every cache usage to v4.

-      - uses: actions/cache@v3
+      - uses: actions/cache@v4
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cee478b and 4834c9c.

📒 Files selected for processing (1)
  • .github/workflows/ci.yml (2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/ci.yml

70-70: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: test-android

@islameldesoky95 islameldesoky95 force-pushed the chore/MSDK-2613-fix-ios-sample-tests branch 2 times, most recently from 93e385d to 91a0235 Compare October 9, 2025 08:59
@islameldesoky95 islameldesoky95 force-pushed the chore/MSDK-2613-fix-ios-sample-tests branch from 7502c63 to 9029881 Compare October 9, 2025 09:19
@islameldesoky95 islameldesoky95 force-pushed the chore/MSDK-2613-fix-ios-sample-tests branch 3 times, most recently from 6ae5e62 to 8858029 Compare October 9, 2025 09:52
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)

16-21: Update deprecated GitHub Actions to v4 (prevents Node 12/16 runtime failures)

These versions are on old Node runtimes and may fail. Bump to v4 and add distribution to setup-java.

-      - uses: actions/checkout@v1
+      - uses: actions/checkout@v4
...
-        uses: actions/setup-node@v2
+        uses: actions/setup-node@v4
...
-      - name: Checkout
-        uses: actions/checkout@v2
+      - name: Checkout
+        uses: actions/checkout@v4
...
-      - name: Setup Node
-        uses: actions/setup-node@v2
+      - name: Setup Node
+        uses: actions/setup-node@v4
...
-      - name: Setup Node
-        uses: actions/setup-node@v2
+      - name: Setup Node
+        uses: actions/setup-node@v4
...
-      - name: Java 17
-        uses: actions/setup-java@v1
-        with:
-          java-version: 17
+      - name: Java 17
+        uses: actions/setup-java@v4
+        with:
+          distribution: temurin
+          java-version: 17

Also applies to: 48-54, 95-103

🧹 Nitpick comments (4)
.github/workflows/ci.yml (4)

55-59: Use ruby/setup-ruby bundler-cache and drop manual bundle install

Let the action manage Bundler caching and remove the extra step. Also point the step at the sample folder so it picks up the correct Gemfile.

-      - name: Setup Ruby
-        uses: ruby/setup-ruby@v1
-        with:
-          ruby-version: '3.2'
+      - name: Setup Ruby
+        working-directory: ./sample
+        uses: ruby/setup-ruby@v1
+        with:
+          ruby-version: '3.2'
+          bundler-cache: true

-      - name: Bundle install (Ruby gems)
-        working-directory: ./sample
-        run: |
-          bundle config set path 'vendor/bundle'
-          bundle install --jobs 4 --retry 3
+      # Bundler install handled by ruby/setup-ruby bundler-cache

Also applies to: 70-75


76-79: Tighten pod install step working directory

Avoid inline cd; point the step to ios directly.

-      - name: Install pods (via bundler)
-        working-directory: ./sample
-        run: cd ios && bundle exec pod install --repo-update
+      - name: Install pods (via bundler)
+        working-directory: ./sample/ios
+        run: bundle exec pod install --repo-update

82-82: Harden xcodebuild destination; drop deprecated -sdk

Use OS=latest to avoid mismatches and rely on -destination instead of -sdk.

-        run: cd ios && xcodebuild -workspace sample.xcworkspace -scheme "sample" -sdk iphonesimulator -destination 'platform=iOS Simulator,name=iPhone 15' test
+        run: cd ios && xcodebuild -workspace sample.xcworkspace -scheme "sample" -destination 'platform=iOS Simulator,OS=latest,name=iPhone 15' -destination-timeout 120 test

Optionally add a step before tests to list available simulators for debugging:

xcrun simctl list devices

Please confirm that “iPhone 15” is present on Xcode 16.1 runner.


88-89: Unused env variable GRADLE_VERSION

It’s defined but not used. Remove or wire it into Gradle if intended.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6656086 and 8858029.

⛔ Files ignored due to path filters (3)
  • package-lock.json is excluded by !**/package-lock.json
  • sample/ios/Podfile.lock is excluded by !**/*.lock
  • sample/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (3)
  • .github/workflows/ci.yml (3 hunks)
  • sample/Gemfile (1 hunks)
  • sample/ios/sample.xcodeproj/project.pbxproj (16 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: test-android
🔇 Additional comments (2)
.github/workflows/ci.yml (2)

40-46: macOS 14 + Xcode 16.1: LGTM

Good upgrade for iOS tests.


4-4: NODE_JS_VERSION is valid: 20.19.5 is an official LTS release; no change needed.

@islameldesoky95 islameldesoky95 force-pushed the chore/MSDK-2613-fix-ios-sample-tests branch from 8858029 to 947d08c Compare October 9, 2025 10:20
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
sample/ios/sample.xcodeproj/project.pbxproj (1)

630-630: Fix the invalid iOS deployment target (previously flagged).

The IPHONEOS_DEPLOYMENT_TARGET = 26.0 setting on lines 630 and 669 remains unfixed from the previous review. iOS 26.0 does not exist. This must be changed to 15.1 to match the main app target before the build can succeed.

Apply this diff to both Debug and Release configurations:

-				IPHONEOS_DEPLOYMENT_TARGET = 26.0;
+				IPHONEOS_DEPLOYMENT_TARGET = 15.1;

Also applies to: 669-669

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8858029 and 947d08c.

⛔ Files ignored due to path filters (3)
  • package-lock.json is excluded by !**/package-lock.json
  • sample/ios/Podfile.lock is excluded by !**/*.lock
  • sample/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (3)
  • .github/workflows/ci.yml (3 hunks)
  • sample/Gemfile (1 hunks)
  • sample/ios/sample.xcodeproj/project.pbxproj (16 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • sample/Gemfile
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test-android
  • GitHub Check: test-ios
🔇 Additional comments (7)
sample/ios/sample.xcodeproj/project.pbxproj (7)

6-16: LGTM!

The objectVersion update and framework references for both the main app and test target are correctly configured for CocoaPods integration.


18-76: LGTM!

The container proxy, file references, synchronized groups, and framework build phases are properly configured to link the sampleTests target to the main sample target.


78-188: LGTM!

The group structure and native target definition for sampleTests are properly configured, using fileSystemSynchronizedGroups for automatic source file management.


226-358: LGTM!

The resource, shell script, and source build phases are correctly configured for both targets, with CocoaPods integration properly set up.


361-367: LGTM!

The target dependency correctly establishes that sampleTests depends on the sample target.


369-610: LGTM!

The build configurations for the main sample target are properly set with correct deployment target (15.1) and standard build settings.


688-716: LGTM!

The configuration lists are properly set up for both targets with Release as the default configuration.

Comment on lines +195 to +203
LastSwiftUpdateCheck = 2600;
LastUpgradeCheck = 1210;
TargetAttributes = {
13B07F861A680F5B00A75B9A = {
LastSwiftMigration = 1120;
};
B876D4782E95685D00879086 = {
CreatedOnToolsVersion = 26.0.1;
TestTargetID = 13B07F861A680F5B00A75B9A;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fix invalid Xcode version references.

Lines 195 and 202 reference non-existent Xcode version 26.0/26.0.1. As of October 2025, Xcode versions are in the 15.x-16.x range. These should likely be 16.0 and 16.0.1 respectively.

Apply this diff to correct the version references:

-				LastSwiftUpdateCheck = 2600;
+				LastSwiftUpdateCheck = 1600;
-					CreatedOnToolsVersion = 26.0.1;
+					CreatedOnToolsVersion = 16.0.1;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
LastSwiftUpdateCheck = 2600;
LastUpgradeCheck = 1210;
TargetAttributes = {
13B07F861A680F5B00A75B9A = {
LastSwiftMigration = 1120;
};
B876D4782E95685D00879086 = {
CreatedOnToolsVersion = 26.0.1;
TestTargetID = 13B07F861A680F5B00A75B9A;
LastSwiftUpdateCheck = 1600;
LastUpgradeCheck = 1210;
TargetAttributes = {
13B07F861A680F5B00A75B9A = {
LastSwiftMigration = 1120;
};
B876D4782E95685D00879086 = {
CreatedOnToolsVersion = 16.0.1;
TestTargetID = 13B07F861A680F5B00A75B9A;
🤖 Prompt for AI Agents
In sample/ios/sample.xcodeproj/project.pbxproj around lines 195 to 203 the file
contains invalid Xcode version references "26.0" and "26.0.1"; update those to
realistic current versions (e.g., "16.0" and "16.0.1") so
LastSwiftUpdateCheck/LastUpgradeCheck and CreatedOnToolsVersion fields reference
the correct Xcode/toolchain versions; make the replacements consistently at
lines 195 and 202 and preserve surrounding formatting.

@islameldesoky95 islameldesoky95 force-pushed the chore/MSDK-2613-fix-ios-sample-tests branch from 947d08c to 367f279 Compare October 9, 2025 10:26
@islameldesoky95 islameldesoky95 force-pushed the chore/MSDK-2613-fix-ios-sample-tests branch from 367f279 to 150f28f Compare October 9, 2025 10:33
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

♻️ Duplicate comments (1)
sample/Gemfile (1)

6-10: Pin git-sourced gems to a tag/ref for reproducible installs; also fix gem ordering.

Using CocoaPods and Xcodeproj from git without a tag/branch/ref is non-deterministic and can break CI. Please pin both to a released tag or commit SHA. Also, RuboCop flags ordering: activesupport should be listed before cocoapods.

Example:

- gem 'cocoapods', git: 'https://github.com/CocoaPods/CocoaPods.git'
- gem 'activesupport', '>= 6.1.7.5', '!= 7.1.0'
- gem 'xcodeproj', git: 'https://github.com/CocoaPods/Xcodeproj.git'
+ gem 'activesupport', '>= 6.1.7.5', '!= 7.1.0'
+ gem 'cocoapods', git: 'https://github.com/CocoaPods/CocoaPods.git', tag: '<validated-tag-or-ref>'
+ gem 'xcodeproj', git: 'https://github.com/CocoaPods/Xcodeproj.git', tag: '<validated-tag-or-ref>'

Optionally align the declared Ruby version with CI (you install Ruby 3.2); setting ruby "~> 3.2" here reduces surprises.

🧹 Nitpick comments (4)
.github/workflows/ci.yml (2)

80-84: Use a single Xcode version for the entire job (prefer 16.1).

Setting Xcode to 15.4, then switching to 16.1 is slow and can cause mismatches. Given the Podfile already patches xcodeproj for objectVersion 70, run the whole job on 16.1 and drop the first setup-xcode step.

-      - uses: maxim-lobanov/setup-xcode@v1
-        with:
-          xcode-version: '15.4'
...
-      - name: Switch to Xcode 16.1 for build & test
-        uses: maxim-lobanov/setup-xcode@v1
-        with:
-          xcode-version: '16.1'
+      - uses: maxim-lobanov/setup-xcode@v1
+        with:
+          xcode-version: '16.1'

Also applies to: 43-46


55-59: Enable Bundler cache via ruby/setup-ruby and drop the manual bundle install step.

Use bundler-cache for faster, simpler CI.

-      - name: Setup Ruby
-        uses: ruby/setup-ruby@v1
-        with:
-          ruby-version: '3.2'
+      - name: Setup Ruby
+        uses: ruby/setup-ruby@v1
+        with:
+          ruby-version: '3.2'
+          bundler-cache: true

-      - name: Bundle install (Ruby gems)
-        working-directory: ./sample
-        run: |
-          bundle config set path 'vendor/bundle'
-          bundle install --jobs 4 --retry 3

Also applies to: 70-75

sample/ios/Podfile (2)

15-29: Move pre_install to top-level for clarity and to avoid duplicate hook definitions.

pre_install works here, but it’s conventionally defined at the Podfile root. Move it above target 'sample' to prevent surprises if new targets are added.

-target 'sample' do
-  ...
-  pre_install do |installer|
+pre_install do |installer|
     begin
       require 'xcodeproj'
       if defined?(Xcodeproj::Constants::COMPATIBILITY_VERSION_BY_OBJECT_VERSION)
         Xcodeproj::Constants::COMPATIBILITY_VERSION_BY_OBJECT_VERSION[70] ||= 'Xcode 16.0'
       end
       if defined?(Xcodeproj::Constants::OBJECT_VERSION_BY_COMPATIBILITY_VERSION)
         Xcodeproj::Constants::OBJECT_VERSION_BY_COMPATIBILITY_VERSION['Xcode 16.0'] ||= 70
       end
     rescue => e
       Pod::UI.puts("[Podfile] Warning: xcodeproj patch failed: #{e}")
     end
-  end
+end
+
+target 'sample' do
   ...

54-65: C++ settings update is safe; consider gnu++20 if you hit compatibility issues.

The inherited c++20/libc++ flags are fine. If any Pods expect GNU extensions, switch CLANG_CXX_LANGUAGE_STANDARD to gnu++20.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 367f279 and 150f28f.

⛔ Files ignored due to path filters (3)
  • package-lock.json is excluded by !**/package-lock.json
  • sample/ios/Podfile.lock is excluded by !**/*.lock
  • sample/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (4)
  • .github/workflows/ci.yml (3 hunks)
  • sample/Gemfile (1 hunks)
  • sample/ios/Podfile (3 hunks)
  • sample/ios/sample.xcodeproj/project.pbxproj (16 hunks)
🧰 Additional context used
🪛 RuboCop (1.81.1)
sample/Gemfile

[convention] 8-8: Gems should be sorted in an alphabetical order within their section of the Gemfile. Gem activesupport should appear before cocoapods.

(Bundler/OrderedGems)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test-ios
  • GitHub Check: test-android
🔇 Additional comments (1)
sample/ios/Podfile (1)

42-45: Test target wiring looks good.

sampleTests correctly inherits search paths from the app target.

Comment on lines 69 to 87
- name: Run tests
working-directory: ./sample
run: cd ios && xcodebuild -workspace sample.xcworkspace -scheme "sample" -sdk iphonesimulator -destination 'platform=iOS Simulator,name=iPhone 14' test
run: cd ios && xcodebuild -workspace sample.xcworkspace -scheme "sample" -sdk iphonesimulator -destination 'platform=iOS Simulator,name=iPhone 15' test
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Ensure the scheme runs the new test target.

Verify that the "sample" scheme includes "sampleTests" so xcodebuild actually runs tests (not 0 tests).

Run this to confirm a shared scheme exists and references sampleTests:


🏁 Script executed:

#!/bin/bash
# List schemes and check testable references
fd -t f -e xcscheme sample/ios | while read f; do
  echo "Scheme: $f"
  rg -n 'TestableReference' -nH "$f" | sed 's/^/  /'
  rg -n 'BlueprintName="sampleTests"' -nH "$f" | sed 's/^/    /'
done

Length of output: 345


🏁 Script executed:

#!/bin/bash
# List schemes and check testable references under sample/ios
fd -t f -e xcscheme . sample/ios | while IFS= read -r f; do
  echo "Scheme: $f"
  rg -n 'TestableReference' -nH "$f" | sed 's/^/  /'
  rg -n 'BlueprintName=\"sampleTests\"' -nH "$f" | sed 's/^/    /'
done

Length of output: 920


🏁 Script executed:

#!/bin/bash
# List schemes and check testable references under sample/ios with correct flags
fd -t f -e xcscheme . sample/ios | while IFS= read -r f; do
  echo "Scheme: $f"
  rg -nH 'TestableReference' "$f" | sed 's/^/  /'
  rg -nH 'BlueprintName="sampleTests"' "$f" | sed 's/^/    /'
done

Length of output: 276


Include sampleTests in the shared ‘sample’ scheme
The scheme’s sample.xcscheme has TestableReference entries but no BlueprintName="sampleTests", so xcodebuild will run 0 tests. Add a TestableReference for “sampleTests” in sample/ios/sample.xcodeproj/xcshareddata/xcschemes/sample.xcscheme.

🤖 Prompt for AI Agents
.github/workflows/ci.yml lines 85-87: the CI step invokes xcodebuild on the
shared “sample” scheme but that scheme’s sample.xcscheme is missing a
TestableReference for the “sampleTests” target so xcodebuild runs zero tests;
open sample/ios/sample.xcodeproj/xcshareddata/xcschemes/sample.xcscheme and add
a TestableReference entry with BlueprintName="sampleTests" (and the matching
BuildableReference attributes for the sampleTests target) inside the
<TestAction> so the tests are included when running xcodebuild.

Comment on lines +195 to 205
LastSwiftUpdateCheck = 2600;
LastUpgradeCheck = 1210;
TargetAttributes = {
13B07F861A680F5B00A75B9A = {
LastSwiftMigration = 1120;
};
B876D4782E95685D00879086 = {
CreatedOnToolsVersion = 26.0.1;
TestTargetID = 13B07F861A680F5B00A75B9A;
};
};
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Correct toolchain metadata to realistic Xcode versions.

These values look auto-bumped incorrectly and can cause Xcode to nag or mis-handle upgrades. Set them to Xcode 16 equivalents and align compatibilityVersion.

-                LastSwiftUpdateCheck = 2600;
+                LastSwiftUpdateCheck = 1600;
...
-                        CreatedOnToolsVersion = 26.0.1;
+                        CreatedOnToolsVersion = 16.1;
-            compatibilityVersion = "Xcode 12.0";
+            compatibilityVersion = "Xcode 16.0";

Also applies to: 208-208

🤖 Prompt for AI Agents
In sample/ios/sample.xcodeproj/project.pbxproj around lines 195-205 (also update
line 208), the toolchain metadata fields (LastSwiftUpdateCheck,
LastUpgradeCheck, CreatedOnToolsVersion, LastSwiftMigration and
compatibilityVersion if present) are set to unrealistic/auto-bumped values;
replace them with Xcode 16-equivalent values (use CreatedOnToolsVersion = 16.0,
LastSwiftUpdateCheck/LastSwiftMigration/LastUpgradeCheck to the corresponding
Xcode 16 numeric codes) and ensure the target compatibilityVersion aligns with
Xcode 16 to stop Xcode upgrade nags and keep project metadata consistent.

Comment on lines +352 to +358
B876D4752E95685D00879086 /* Sources */ = {
isa = PBXSourcesBuildPhase;
buildActionMask = 2147483647;
files = (
);
runOnlyForDeploymentPostprocessing = 0;
};
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Confirm test sources are included and scheme runs them.

PBXSourcesBuildPhase for sampleTests shows no files here; with filesystem-synced groups and Xcode 16 it may still be fine, but please verify tests are discovered and run in CI (>0 tests).

Also applies to: 169-188


🏁 Script executed:

#!/bin/bash
# Verify scheme references sampleTests and that tests are not 0
fd -t f -e xcscheme sample/ios | while read f; do
  echo "Scheme: $f"
  rg -n 'BlueprintName="sampleTests"' "$f" || echo "  → sampleTests not referenced"
done

Length of output: 345


🏁 Script executed:

#!/bin/bash
# Locate all Xcode scheme files under sample/ios and verify sampleTests target is referenced
find sample/ios -type f -name '*.xcscheme' -print0 | while IFS= read -r -d '' f; do
  echo "Scheme: $f"
  grep -n 'BlueprintName="sampleTests"' "$f" || echo "  → sampleTests not referenced"
done

Length of output: 105


Include sampleTests in the shared scheme
The shared sample.xcscheme (xcshareddata/xcschemes/sample.xcscheme) doesn’t reference the sampleTests target—so no tests run (0 tests). Add BlueprintName="sampleTests" under the <TestAction> to ensure tests are discovered and executed in CI.

🤖 Prompt for AI Agents
In sample/ios/sample.xcodeproj/project.pbxproj around lines 352-358 the shared
scheme is missing the test target reference; open
sample/ios/xcshareddata/xcschemes/sample.xcscheme, locate the <TestAction>
element and add a TestableReference entry that references the sampleTests target
by including BlueprintName="sampleTests" (and the appropriate
referencedContainer if missing), save and commit the updated xcshared scheme so
CI discovers and runs the tests.

Comment on lines 611 to 647
B876D47F2E95685D00879086 /* Debug */ = {
isa = XCBuildConfiguration;
baseConfigurationReference = DF2117459BB74FA9D6B0EE3A /* Pods-sampleTests.debug.xcconfig */;
buildSettings = {
ASSETCATALOG_COMPILER_GENERATE_SWIFT_ASSET_SYMBOL_EXTENSIONS = YES;
BUNDLE_LOADER = "$(TEST_HOST)";
CLANG_ANALYZER_NONNULL = YES;
CLANG_ANALYZER_NUMBER_OBJECT_CONVERSION = YES_AGGRESSIVE;
CLANG_CXX_LANGUAGE_STANDARD = "gnu++20";
CLANG_ENABLE_OBJC_WEAK = YES;
CLANG_WARN_DOCUMENTATION_COMMENTS = YES;
CLANG_WARN_UNGUARDED_AVAILABILITY = YES_AGGRESSIVE;
CODE_SIGN_STYLE = Automatic;
CURRENT_PROJECT_VERSION = 1;
DEBUG_INFORMATION_FORMAT = dwarf;
DEVELOPMENT_TEAM = 4GB94SV7D4;
ENABLE_USER_SCRIPT_SANDBOXING = YES;
GCC_C_LANGUAGE_STANDARD = gnu17;
GENERATE_INFOPLIST_FILE = YES;
IPHONEOS_DEPLOYMENT_TARGET = 26.0;
LOCALIZATION_PREFERS_STRING_CATALOGS = YES;
MARKETING_VERSION = 1.0;
MTL_ENABLE_DEBUG_INFO = INCLUDE_SOURCE;
MTL_FAST_MATH = YES;
PRODUCT_BUNDLE_IDENTIFIER = com.usercentrics.sampleTest.sampleTests;
PRODUCT_NAME = "$(TARGET_NAME)";
STRING_CATALOG_GENERATE_SYMBOLS = NO;
SWIFT_ACTIVE_COMPILATION_CONDITIONS = "DEBUG $(inherited)";
SWIFT_APPROACHABLE_CONCURRENCY = YES;
SWIFT_EMIT_LOC_STRINGS = NO;
SWIFT_OPTIMIZATION_LEVEL = "-Onone";
SWIFT_UPCOMING_FEATURE_MEMBER_IMPORT_VISIBILITY = YES;
SWIFT_VERSION = 5.0;
TARGETED_DEVICE_FAMILY = "1,2";
TEST_HOST = "$(BUILT_PRODUCTS_DIR)/sample.app/$(BUNDLE_EXECUTABLE_FOLDER_PATH)/sample";
};
name = Debug;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fix invalid deployment target for sampleTests (26.0 → 15.1).

iOS 26.0 doesn’t exist; this will fail builds. Align with the app’s 15.1.

-                IPHONEOS_DEPLOYMENT_TARGET = 26.0;
+                IPHONEOS_DEPLOYMENT_TARGET = 15.1;

Apply to both Debug and Release configs shown above.

Also applies to: 649-685

🤖 Prompt for AI Agents
In sample/ios/sample.xcodeproj/project.pbxproj around lines 611-647 and also
apply to 649-685, the IPHONEOS_DEPLOYMENT_TARGET is incorrectly set to 26.0;
change it to 15.1 in both the Debug and Release XCBuildConfiguration blocks
(update the IPHONEOS_DEPLOYMENT_TARGET value to "15.1" for each configuration).

⚠️ Potential issue | 🟠 Major

Disable code signing for the test bundle to avoid CI/team provisioning issues.

The test target sets CODE_SIGN_STYLE = Automatic and a specific DEVELOPMENT_TEAM. For simulator builds, disable signing to keep CI stable.

             CODE_SIGN_STYLE = Automatic;
+            CODE_SIGNING_ALLOWED = NO;
+            CODE_SIGNING_REQUIRED = NO;
+            CODE_SIGN_IDENTITY = "";
-            DEVELOPMENT_TEAM = 4GB94SV7D4;
+            DEVELOPMENT_TEAM = "";

Apply to both Debug and Release of sampleTests. Alternatively, scope these to the iphonesimulator SDK via conditional settings.

Also applies to: 649-685

🤖 Prompt for AI Agents
In sample/ios/sample.xcodeproj/project.pbxproj around lines 611-647 (and also
apply to 649-685), the test target config contains CODE_SIGN_STYLE = Automatic
and a DEVELOPMENT_TEAM which forces signing; disable signing for the test bundle
by adding/setting CODE_SIGNING_ALLOWED = NO (and/or CODE_SIGN_STYLE = None) and
remove or clear DEVELOPMENT_TEAM for both Debug and Release configurations of
sampleTests; alternatively scope this change to the simulator only by adding a
conditional setting (e.g., for SDKROOT == iphonesimulator) that sets
CODE_SIGNING_ALLOWED = NO so simulator CI builds won’t require provisioning.

@islameldesoky95 islameldesoky95 force-pushed the chore/MSDK-2613-fix-ios-sample-tests branch 2 times, most recently from 22597bc to 4ac3ffc Compare October 9, 2025 10:54
@islameldesoky95 islameldesoky95 force-pushed the chore/MSDK-2613-fix-ios-sample-tests branch from 4ac3ffc to 1de1a89 Compare October 9, 2025 12:13
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (3)
sample/ios/sample.xcodeproj/project.pbxproj (3)

623-626: Consider disabling code signing for simulator test builds.

The test target uses automatic code signing with a hardcoded development team:

CODE_SIGN_STYLE = Automatic;
DEVELOPMENT_TEAM = 4GB94SV7D4;

For simulator builds in CI, code signing is unnecessary and the hardcoded team ID will cause issues for developers outside your team.

Consider applying this diff to both Debug and Release configurations:

				CODE_SIGN_STYLE = Automatic;
+				CODE_SIGNING_ALLOWED = NO;
+				CODE_SIGNING_REQUIRED = NO;
+				CODE_SIGN_IDENTITY = "";
-				DEVELOPMENT_TEAM = 4GB94SV7D4;
+				DEVELOPMENT_TEAM = "";

Alternatively, scope this to simulator SDK only by using conditional settings:

"CODE_SIGNING_ALLOWED[sdk=iphonesimulator*]" = NO;
"CODE_SIGN_IDENTITY[sdk=iphonesimulator*]" = "";

Also applies to: 661-665


167-188: Verify the test target is included in the shared Xcode scheme.

The test target is properly configured in the project file, but past comments indicate it may not be included in the shared scheme (xcshareddata/xcschemes/sample.xcscheme). Without scheme inclusion, tests won't run in CI (0 tests discovered).

Verify the test target is referenced in the scheme:

#!/bin/bash
# Check if sampleTests is included in the shared scheme
if [ -f "sample/ios/sample.xcodeproj/xcshareddata/xcschemes/sample.xcscheme" ]; then
  echo "=== Checking for sampleTests in shared scheme ==="
  grep -n 'sampleTests' sample/ios/sample.xcodeproj/xcshareddata/xcschemes/sample.xcscheme || echo "❌ sampleTests not found in scheme"
  
  echo -e "\n=== Checking TestAction configuration ==="
  rg -A 10 '<TestAction' sample/ios/sample.xcodeproj/xcshareddata/xcschemes/sample.xcscheme || echo "TestAction not found"
else
  echo "❌ Shared scheme file not found"
  echo "Expected location: sample/ios/sample.xcodeproj/xcshareddata/xcschemes/sample.xcscheme"
fi

195-205: Fix invalid Xcode version metadata.

The toolchain metadata still references non-existent Xcode versions:

  • Line 195: LastSwiftUpdateCheck = 2600; should be 1600
  • Line 202: CreatedOnToolsVersion = 26.0.1; should be 16.0.1 or 16.1

As of October 2025, Xcode versions are in the 15.x-16.x range. These incorrect values can cause Xcode upgrade dialogs and project compatibility warnings.

Apply this diff:

-				LastSwiftUpdateCheck = 2600;
+				LastSwiftUpdateCheck = 1600;
-					CreatedOnToolsVersion = 26.0.1;
+					CreatedOnToolsVersion = 16.1;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4ac3ffc and 1de1a89.

📒 Files selected for processing (3)
  • .github/workflows/ci.yml (3 hunks)
  • sample/ios/Podfile (3 hunks)
  • sample/ios/sample.xcodeproj/project.pbxproj (16 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/ci.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test-android
  • GitHub Check: test-ios
🔇 Additional comments (3)
sample/ios/Podfile (1)

29-33: LGTM! Test target properly configured.

The test target correctly uses inherit! :search_paths, which is the recommended approach for CocoaPods test targets. This ensures the test target can access dependencies without duplicating all pod settings.

sample/ios/sample.xcodeproj/project.pbxproj (2)

18-26: LGTM! Test target infrastructure correctly configured.

The test target infrastructure is properly set up:

  • ✅ PBXContainerItemProxy links the test target to the main sample target
  • ✅ PBXFileSystemSynchronizedRootGroup uses modern Xcode file organization
  • ✅ PBXTargetDependency ensures tests run against the correct app target
  • ✅ Test bundle product reference and build phases are correctly defined

Also applies to: 45-57, 167-188, 361-367


33-41: LGTM! CocoaPods integration properly configured.

The CocoaPods configuration correctly includes:

  • ✅ Separate xcconfig file references for both sample and sampleTests targets
  • ✅ Debug and Release configurations for each target
  • ✅ Proper framework references (Pods_sample.framework, Pods_sampleTests.framework)

This aligns with the Podfile changes that define the test target with inherited search paths.

Also applies to: 133-142

config.build_settings['CLANG_CXX_LANGUAGE_STANDARD'] = '$(inherited) c++20'
config.build_settings['CLANG_CXX_LIBRARY'] = '$(inherited) libc++'
config.build_settings['OTHER_CPLUSPLUSFLAGS'] = '$(inherited) -std=c++20 -stdlib=libc++'
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Verify the $(inherited) string concatenation approach.

The build settings use an unusual pattern where $(inherited) is concatenated with values in string format:

config.build_settings['CLANG_CXX_LANGUAGE_STANDARD'] = '$(inherited) c++20'

This syntax is atypical. In Xcode build settings, $(inherited) is typically used in array contexts or through proper variable expansion. When set as a literal string like this, Xcode may not expand it correctly, potentially resulting in the literal text "$(inherited) c++20" rather than the intended inheritance behavior.

Consider these alternatives:

  1. Set directly without inheritance: 'c++20'
  2. Use array-based settings if inheritance is needed
  3. Test that the resulting build settings show the expected values (not the literal $(inherited) string)

Verify the effective build settings after pod install:


🏁 Script executed:

#!/bin/bash
# Check if $(inherited) is being expanded correctly in build settings
cd sample/ios
pod install 2>&1 | head -20

# Extract and display the actual C++ standard setting from the generated xcconfig
echo "=== Checking Pods-sample.debug.xcconfig ==="
grep -E 'CLANG_CXX|OTHER_CPLUSPLUSFLAGS' Pods/Target\ Support\ Files/Pods-sample/Pods-sample.debug.xcconfig || echo "Settings not found in xcconfig"

# Check the project file to see how settings are represented
echo "=== Checking build settings in project.pbxproj ==="
grep -A2 -B2 'CLANG_CXX_LANGUAGE_STANDARD' sample.xcodeproj/project.pbxproj | head -20

Length of output: 942


Use array syntax for inherited build settings in sample/ios/Podfile
String concatenation with $(inherited) isn’t applied correctly. Assign these as arrays so Xcode expands $(inherited) properly:

config.build_settings['CLANG_CXX_LANGUAGE_STANDARD'] = ['$(inherited)', 'c++20']
config.build_settings['CLANG_CXX_LIBRARY']           = ['$(inherited)', 'libc++']
config.build_settings['OTHER_CPLUSPLUSFLAGS']         = ['$(inherited)', '-std=c++20', '-stdlib=libc++']
🤖 Prompt for AI Agents
In sample/ios/Podfile around lines 49 to 51, the build settings currently use
string values with '$(inherited)' which prevents Xcode from expanding inherited
settings; change each assignment to an array where the first element is
'$(inherited)' followed by the new values: set CLANG_CXX_LANGUAGE_STANDARD to an
array with '$(inherited)' and 'c++20', CLANG_CXX_LIBRARY to an array with
'$(inherited)' and 'libc++', and OTHER_CPLUSPLUSFLAGS to an array with
'$(inherited)', '-std=c++20', and '-stdlib=libc++' so Xcode correctly merges
inherited settings.

@islameldesoky95 islameldesoky95 force-pushed the chore/MSDK-2613-fix-ios-sample-tests branch from 1de1a89 to 714cfeb Compare October 9, 2025 12:20
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)

47-52: Update GitHub Actions to Node 20–compatible majors.
actions/checkout@v2 and actions/setup-node@v2 still run on the retired Node 16 runtime; GitHub is actively blocking these invocations, so this job will fail before reaching your new Bundler/pod steps. Please bump to the current majors (e.g. checkout@v4, setup-node@v4) so the workflow actually executes on macOS 14 with Xcode 16.1.

Apply this diff:

-      - name: Checkout
-        uses: actions/checkout@v2
+      - name: Checkout
+        uses: actions/checkout@v4-      - name: Setup Node
-        uses: actions/setup-node@v2
+      - name: Setup Node
+        uses: actions/setup-node@v4
♻️ Duplicate comments (1)
sample/ios/Podfile (1)

73-77: Fix inherited C++ build settings.
Setting CLANG_CXX_LANGUAGE_STANDARD/CLANG_CXX_LIBRARY/OTHER_CPLUSPLUSFLAGS to '$(inherited) …' creates a single literal string (e.g. "gnu++17 c++20"), which breaks the compiler flags. Use array syntax so $(inherited) is merged correctly.

Apply this diff:

-        config.build_settings['CLANG_CXX_LANGUAGE_STANDARD'] = '$(inherited) c++20'
-        config.build_settings['CLANG_CXX_LIBRARY'] = '$(inherited) libc++'
-        config.build_settings['OTHER_CPLUSPLUSFLAGS'] = '$(inherited) -std=c++20 -stdlib=libc++'
+        config.build_settings['CLANG_CXX_LANGUAGE_STANDARD'] = ['$(inherited)', 'c++20']
+        config.build_settings['CLANG_CXX_LIBRARY'] = ['$(inherited)', 'libc++']
+        config.build_settings['OTHER_CPLUSPLUSFLAGS'] = ['$(inherited)', '-std=c++20', '-stdlib=libc++']
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1de1a89 and 714cfeb.

📒 Files selected for processing (3)
  • .github/workflows/ci.yml (3 hunks)
  • sample/ios/Podfile (3 hunks)
  • sample/ios/sample.xcodeproj/project.pbxproj (16 hunks)

@islameldesoky95 islameldesoky95 force-pushed the chore/MSDK-2613-fix-ios-sample-tests branch from 714cfeb to 2909647 Compare October 9, 2025 12:50
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (4)
sample/ios/Podfile (1)

75-77: Use array syntax for inherited build settings.

The string concatenation with $(inherited) won't expand correctly in Xcode. This issue was flagged in a previous review but remains unaddressed.

Apply this diff:

-        config.build_settings['CLANG_CXX_LANGUAGE_STANDARD'] = '$(inherited) c++20'
-        config.build_settings['CLANG_CXX_LIBRARY'] = '$(inherited) libc++'
-        config.build_settings['OTHER_CPLUSPLUSFLAGS'] = '$(inherited) -std=c++20 -stdlib=libc++'
+        config.build_settings['CLANG_CXX_LANGUAGE_STANDARD'] = ['$(inherited)', 'c++20']
+        config.build_settings['CLANG_CXX_LIBRARY'] = ['$(inherited)', 'libc++']
+        config.build_settings['OTHER_CPLUSPLUSFLAGS'] = ['$(inherited)', '-std=c++20', '-stdlib=libc++']
sample/ios/sample.xcodeproj/project.pbxproj (3)

195-195: Correct the invalid Xcode version references.

Lines 195 and 202 still reference non-existent Xcode versions (2600 = 26.0 and 26.0.1). As of October 2025, Xcode versions are in the 15.x-16.x range. These should be 1600 (16.0) and 16.0.1 respectively.

Apply this diff:

-				LastSwiftUpdateCheck = 2600;
+				LastSwiftUpdateCheck = 1600;
-					CreatedOnToolsVersion = 26.0.1;
+					CreatedOnToolsVersion = 16.0.1;

Also applies to: 202-202


352-358: Add test sources to the sampleTests build phase.

The PBXSourcesBuildPhase for sampleTests is empty, so no test files will compile and xcodebuild will run 0 tests. This critical issue was flagged in a previous review but remains unresolved.

Verify that test source files exist and generate the necessary PBXBuildFile and PBXFileReference entries for all Swift test files in the sampleTests directory. Run the following script to identify test files:

#!/bin/bash
# Find all Swift files in the sampleTests directory
fd -e swift . sample/ios/sampleTests 2>/dev/null || echo "No sampleTests directory or no Swift files found"

# Check if the shared scheme includes the test target
if [ -f "sample/ios/sample.xcodeproj/xcshareddata/xcschemes/sample.xcscheme" ]; then
  echo "=== Checking shared scheme for test target reference ==="
  grep -n "sampleTests" sample/ios/sample.xcodeproj/xcshareddata/xcschemes/sample.xcscheme || echo "sampleTests not found in scheme"
else
  echo "No shared scheme file found"
fi

623-626: Disable code signing for the test bundle to avoid CI failures.

The test target sets CODE_SIGN_STYLE = Automatic and includes a hardcoded DEVELOPMENT_TEAM. This will cause provisioning failures in CI. For simulator builds, disable signing.

Apply this diff to both Debug and Release configurations:

 			CODE_SIGN_STYLE = Automatic;
+			CODE_SIGNING_ALLOWED = NO;
+			CODE_SIGNING_REQUIRED = NO;
+			CODE_SIGN_IDENTITY = "";
 			CURRENT_PROJECT_VERSION = 1;
 			DEBUG_INFORMATION_FORMAT = dwarf;
-			DEVELOPMENT_TEAM = 4GB94SV7D4;
+			DEVELOPMENT_TEAM = "";

Alternatively, use a conditional setting to only disable signing for simulator:

"CODE_SIGNING_ALLOWED[sdk=iphonesimulator*]" = NO;

Also applies to: 661-665

🧹 Nitpick comments (1)
sample/ios/Podfile (1)

15-41: Consider documenting the xcodeproj compatibility workaround.

The pre_install block modifies constants at runtime to handle objectVersion 70. While the rescue block prevents hard failures, this approach is brittle and may break with future xcodeproj versions.

Consider adding a comment explaining:

  • Why this workaround is needed
  • Which xcodeproj versions require it
  • Whether there's a minimum xcodeproj version requirement

Example:

# Workaround for xcodeproj < 1.23 which doesn't recognize Xcode 16's objectVersion 70
# Can be removed once the minimum CocoaPods version includes xcodeproj >= 1.23
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 714cfeb and 2909647.

📒 Files selected for processing (3)
  • .github/workflows/ci.yml (3 hunks)
  • sample/ios/Podfile (3 hunks)
  • sample/ios/sample.xcodeproj/project.pbxproj (16 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/ci.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test-ios
  • GitHub Check: test-android
🔇 Additional comments (1)
sample/ios/sample.xcodeproj/project.pbxproj (1)

630-630: LGTM! Deployment target is correct.

The IPHONEOS_DEPLOYMENT_TARGET is now correctly set to 15.1, matching the main app target. This addresses the previous review comment.

Also applies to: 669-669

@islameldesoky95 islameldesoky95 force-pushed the chore/MSDK-2613-fix-ios-sample-tests branch from 2909647 to 4ab64d5 Compare October 9, 2025 12:56
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (4)
sample/ios/Podfile (1)

75-77: Fix: Use array syntax for inherited build settings.

String concatenation with $(inherited) won't expand correctly in Xcode. The build system treats "$(inherited) c++20" as a literal string instead of merging inherited values. Use array syntax so Xcode properly inherits and extends settings.

Apply this diff:

-        config.build_settings['CLANG_CXX_LANGUAGE_STANDARD'] = '$(inherited) c++20'
-        config.build_settings['CLANG_CXX_LIBRARY'] = '$(inherited) libc++'
-        config.build_settings['OTHER_CPLUSPLUSFLAGS'] = '$(inherited) -std=c++20 -stdlib=libc++'
+        config.build_settings['CLANG_CXX_LANGUAGE_STANDARD'] = ['$(inherited)', 'c++20']
+        config.build_settings['CLANG_CXX_LIBRARY'] = ['$(inherited)', 'libc++']
+        config.build_settings['OTHER_CPLUSPLUSFLAGS'] = ['$(inherited)', '-std=c++20', '-stdlib=libc++']

Based on past review comments.

sample/ios/sample.xcodeproj/project.pbxproj (3)

195-204: Fix: Correct invalid Xcode version metadata.

LastSwiftUpdateCheck = 2600 and CreatedOnToolsVersion = 26.0.1 reference non-existent Xcode 26. Xcode 16.1 uses version code 1610. These incorrect values can cause Xcode migration prompts or mishandled project upgrades.

Apply this diff:

-				LastSwiftUpdateCheck = 2600;
+				LastSwiftUpdateCheck = 1610;
-					CreatedOnToolsVersion = 26.0.1;
+					CreatedOnToolsVersion = 16.1;

Based on past review comments.


352-358: Critical: Add test sources to sampleTests target.

The PBXSourcesBuildPhase for sampleTests has no file entries—xcodebuild will produce an empty test bundle and run 0 tests. Add all Swift test files from sample/ios/sampleTests/ to this build phase so the tests actually compile and execute.

Based on past review comments.


626-626: Fix: Disable code signing for simulator test bundle.

Lines 626 and 665 set DEVELOPMENT_TEAM = 4GB94SV7D4 and CODE_SIGN_STYLE = Automatic for the test target. CI builds on simulators don't need signing and this team ID will fail in your CI environment without provisioning profiles.

Apply this diff to both Debug (line 626) and Release (line 665) configurations:

-				DEVELOPMENT_TEAM = 4GB94SV7D4;
+				DEVELOPMENT_TEAM = "";
+				CODE_SIGNING_ALLOWED = NO;
+				CODE_SIGNING_REQUIRED = NO;
+				CODE_SIGN_IDENTITY = "";

Alternatively, use conditional settings scoped to iphonesimulator SDK only.

Based on past review comments.

Also applies to: 665-665

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2909647 and 4ab64d5.

📒 Files selected for processing (3)
  • .github/workflows/ci.yml (3 hunks)
  • sample/ios/Podfile (3 hunks)
  • sample/ios/sample.xcodeproj/project.pbxproj (16 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test-ios
  • GitHub Check: test-android
🔇 Additional comments (2)
.github/workflows/ci.yml (1)

40-82: LGTM! CI workflow updates align with modernized tooling.

The workflow now uses macOS-14 with Xcode 16.1, adds Ruby 3.2 setup, and switches to Bundler-based pod installation. The test destination (iOS 18.1, iPhone 16) matches the Xcode version's available simulators. These changes properly support the new test target infrastructure.

sample/ios/Podfile (1)

15-41: Confirm xcodeproj objectVersion 70 mapping
This pre_install hook patches Xcodeproj for objectVersion 70 (Xcode 16). Check your Gemfile.lock’s xcodeproj version and upstream mapping—if version 70 is already supported, remove this workaround.

Comment on lines +167 to +188
B876D4782E95685D00879086 /* sampleTests */ = {
isa = PBXNativeTarget;
buildConfigurationList = B876D4812E95685D00879086 /* Build configuration list for PBXNativeTarget "sampleTests" */;
buildPhases = (
049004BE5B4531E03A27D676 /* [CP] Check Pods Manifest.lock */,
B876D4752E95685D00879086 /* Sources */,
B876D4762E95685D00879086 /* Frameworks */,
B876D4772E95685D00879086 /* Resources */,
);
buildRules = (
);
dependencies = (
B876D47E2E95685D00879086 /* PBXTargetDependency */,
);
fileSystemSynchronizedGroups = (
B876D47A2E95685D00879086 /* sampleTests */,
);
name = sampleTests;
productName = sampleTests;
productReference = B876D4792E95685D00879086 /* sampleTests.xctest */;
productType = "com.apple.product-type.bundle.unit-test";
};
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify: Ensure sampleTests is included in the shared scheme.

The PBXNativeTarget for sampleTests is properly configured with dependencies and build phases, but past review comments indicate the shared scheme (sample/ios/sample.xcodeproj/xcshareddata/xcschemes/sample.xcscheme) doesn't reference this test target. Without a TestableReference entry in the scheme, xcodebuild won't discover or run the tests.

Check the shared scheme for test target references:

If not found, open sample/ios/sample.xcodeproj/xcshareddata/xcschemes/sample.xcscheme and add a <TestableReference> entry within <TestAction> that references the sampleTests target.

Based on past review comments.


🏁 Script executed:

#!/bin/bash
# Verify sampleTests is referenced in the shared scheme
fd -t f -e xcscheme . sample/ios | while IFS= read -r f; do
  echo "Checking scheme: $f"
  if grep -q 'BlueprintName="sampleTests"' "$f"; then
    echo "  ✓ sampleTests is referenced"
  else
    echo "  ✗ sampleTests is NOT referenced"
  fi
done

Length of output: 117


Include sampleTests in shared scheme
Add a <TestableReference> entry for sampleTests in sample/ios/sample.xcodeproj/xcshareddata/xcschemes/sample.xcscheme under the <TestAction> section so xcodebuild can discover and run the tests.

🤖 Prompt for AI Agents
In sample/ios/sample.xcodeproj/xcshareddata/xcschemes/sample.xcscheme (add under
the <TestAction> section), include a <TestableReference> element that references
the sampleTests target so xcodebuild can discover and run the tests; set its
TestableIdentifier/targetIdentifier to the PBXNativeTarget GUID
B876D4782E95685D00879086 (sampleTests), point to the correct target/product in
the scheme (matching sampleTests.xctest), and include attributes like
skipped="false" and selected="YES" (or selected="true") as used by other
testable entries in the file.

@islameldesoky95 islameldesoky95 force-pushed the chore/MSDK-2613-fix-ios-sample-tests branch from 4ab64d5 to b770f3a Compare October 9, 2025 13:18
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (2)
sample/ios/Podfile (1)

75-77: Fix inherited C++ build settings assignment

'$(inherited) c++20' (and the similar lines below) is stored as a single literal value, so Xcode never expands $(inherited) and the resulting setting becomes invalid. Assign each key an array—first $(inherited), then the desired value—so the inheritance actually works.

-        config.build_settings['CLANG_CXX_LANGUAGE_STANDARD'] = '$(inherited) c++20'
-        config.build_settings['CLANG_CXX_LIBRARY'] = '$(inherited) libc++'
-        config.build_settings['OTHER_CPLUSPLUSFLAGS'] = '$(inherited) -std=c++20 -stdlib=libc++'
+        config.build_settings['CLANG_CXX_LANGUAGE_STANDARD'] = ['$(inherited)', 'c++20']
+        config.build_settings['CLANG_CXX_LIBRARY']           = ['$(inherited)', 'libc++']
+        config.build_settings['OTHER_CPLUSPLUSFLAGS']        = ['$(inherited)', '-std=c++20', '-stdlib=libc++']
sample/ios/sample.xcodeproj/project.pbxproj (1)

624-666: Disable signing for the test bundle

The test target still forces CODE_SIGN_STYLE = Automatic with DEVELOPMENT_TEAM = 4GB94SV7D4. Anyone without that team (CI included) can’t build the tests. Clear the team and disable signing for both Debug and Release:

-                CODE_SIGN_STYLE = Automatic;
-                DEVELOPMENT_TEAM = 4GB94SV7D4;
+                CODE_SIGN_STYLE = Automatic;
+                CODE_SIGNING_ALLOWED = NO;
+                CODE_SIGNING_REQUIRED = NO;
+                CODE_SIGN_IDENTITY = "";
+                DEVELOPMENT_TEAM = "";

Repeat the diff for the Release configuration.

Also applies to: 662-683

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4ab64d5 and b770f3a.

📒 Files selected for processing (3)
  • .github/workflows/ci.yml (3 hunks)
  • sample/ios/Podfile (3 hunks)
  • sample/ios/sample.xcodeproj/project.pbxproj (16 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/ci.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test-ios
  • GitHub Check: test-android

Comment on lines +352 to +357
B876D4752E95685D00879086 /* Sources */ = {
isa = PBXSourcesBuildPhase;
buildActionMask = 2147483647;
files = (
);
runOnlyForDeploymentPostprocessing = 0;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Add the test sources to sampleTests

PBXSourcesBuildPhase for sampleTests is empty, so the target compiles zero files and produces an empty test bundle—no XCTest cases will ever run. Add every Swift test/support file under sample/ios/sampleTests to this build phase (Xcode will generate the matching PBXBuildFile entries once you tick their target membership).

🤖 Prompt for AI Agents
In sample/ios/sample.xcodeproj/project.pbxproj around lines 352-357, the
PBXSourcesBuildPhase for the sampleTests target is empty so the test target
compiles no files; fix by adding all Swift test and support file references from
sample/ios/sampleTests into this PBXSourcesBuildPhase's files array (or open the
project in Xcode and check target membership for each file under sampleTests so
Xcode creates the corresponding PBXBuildFile entries and populates the build
phase), ensuring each file reference UUID is present in the files list for that
PBXSourcesBuildPhase.

@islameldesoky95 islameldesoky95 force-pushed the chore/MSDK-2613-fix-ios-sample-tests branch from b770f3a to 8cb5f52 Compare October 9, 2025 14:05
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (4)
sample/ios/sample.xcodeproj/project.pbxproj (3)

352-358: Verify test sources are included via file system synchronization.

The PBXSourcesBuildPhase for sampleTests is empty, but the target uses fileSystemSynchronizedGroups (line 46-56) which may automatically sync Swift files in Xcode 16. Verify that tests compile and discover properly in both Xcode and CI. If tests don't compile, manually add the test sources to this build phase.

Run this script to check if test files are properly referenced:

#!/bin/bash
# Check if test files exist and verify they're being picked up
echo "Test files in sampleTests directory:"
fd -e swift . sample/ios/sampleTests | head -10

echo ""
echo "Attempting to build sampleTests in CI would reveal if files are compiled."
echo "Monitor the CI test step for compilation errors."

Based on past review comments.


195-203: Correct invalid Xcode version references.

Lines 195 and 202 reference non-existent Xcode versions "2600" and "26.0.1". These should be updated to realistic Xcode 16 equivalents:

-				LastSwiftUpdateCheck = 2600;
+				LastSwiftUpdateCheck = 1600;
-					CreatedOnToolsVersion = 26.0.1;
+					CreatedOnToolsVersion = 16.1;

Based on past review comments.


611-647: Disable code signing for the test bundle to avoid CI failures.

The test target has CODE_SIGN_STYLE = Automatic and a hardcoded DEVELOPMENT_TEAM = 4GB94SV7D4. For simulator builds in CI, disable signing to prevent provisioning issues:

 				CODE_SIGN_STYLE = Automatic;
+				CODE_SIGNING_ALLOWED = NO;
+				CODE_SIGNING_REQUIRED = NO;
+				CODE_SIGN_IDENTITY = "";
-				DEVELOPMENT_TEAM = 4GB94SV7D4;
+				DEVELOPMENT_TEAM = "";

Apply to both Debug (line 611-647) and Release (line 649-685) configurations.

Based on past review comments.

sample/ios/Podfile (1)

45-50: Use array syntax for inherited build settings.

The string concatenation with $(inherited) (e.g., '$(inherited) c++20') prevents Xcode from properly expanding inherited values. Use array syntax instead so Xcode correctly merges settings:

config.build_settings['CLANG_CXX_LANGUAGE_STANDARD'] = ['$(inherited)', 'c++20']
config.build_settings['CLANG_CXX_LIBRARY'] = ['$(inherited)', 'libc++']
config.build_settings['OTHER_CPLUSPLUSFLAGS'] = ['$(inherited)', '-std=c++20', '-stdlib=libc++']

Based on past review comments.

🧹 Nitpick comments (1)
sample/Gemfile (1)

7-17: Alphabetize gems for consistency.

RuboCop convention suggests sorting gems alphabetically within their section. Consider reordering:

  • Move activesupport before xcodeproj (lines 8-9)
  • Move benchmark before logger (lines 13-14)

This improves readability and helps prevent duplicate entries.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b770f3a and 7ba3f91.

📒 Files selected for processing (4)
  • .github/workflows/ci.yml (3 hunks)
  • sample/Gemfile (1 hunks)
  • sample/ios/Podfile (2 hunks)
  • sample/ios/sample.xcodeproj/project.pbxproj (16 hunks)
🧰 Additional context used
🪛 RuboCop (1.81.1)
sample/Gemfile

[convention] 9-9: Gems should be sorted in an alphabetical order within their section of the Gemfile. Gem activesupport should appear before xcodeproj.

(Bundler/OrderedGems)


[convention] 15-15: Gems should be sorted in an alphabetical order within their section of the Gemfile. Gem benchmark should appear before logger.

(Bundler/OrderedGems)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test-ios
  • GitHub Check: test-android
🔇 Additional comments (10)
sample/Gemfile (3)

4-4: LGTM!

Ruby version requirement correctly aligns with the CI workflow changes that set up Ruby 3.2.


7-10: LGTM!

Gem version constraints have been updated appropriately. The use of pessimistic version operators (~>) for CocoaPods and Xcodeproj ensures compatibility while allowing patch updates.


17-17: LGTM!

The addition of xcpretty aligns with the CI workflow changes that format xcodebuild test output.

.github/workflows/ci.yml (3)

40-45: LGTM!

The upgrade to macOS-14 and Xcode 16.1 ensures the CI environment supports the latest Swift and iOS features required by the test suite.


55-58: LGTM!

Ruby 3.2 setup correctly aligns with the updated Gemfile requirements and supports Bundler-based pod installation.


70-78: LGTM!

Bundler-based pod installation with vendored gems ensures reproducible builds and consistent dependency versions across CI and local environments.

sample/ios/Podfile (2)

1-6: LGTM!

The updated react_native_pods.rb resolution using Node's require.resolve with the paths option properly supports monorepo and hoisting configurations.


27-30: LGTM!

The test target correctly uses inherit! :search_paths to avoid inheriting problematic build settings while maintaining access to necessary header paths.

sample/ios/sample.xcodeproj/project.pbxproj (2)

167-188: LGTM!

The sampleTests target is properly configured with correct build phases, dependencies on the main app target, and file system synchronized groups for Xcode 16 compatibility.


630-630: LGTM!

The deployment target is correctly set to 15.1, matching the main app target and supporting the required iOS features.

Also applies to: 669-669

Comment on lines 69 to 82
- name: Run tests
working-directory: ./sample
run: cd ios && xcodebuild -workspace sample.xcworkspace -scheme "sample" -sdk iphonesimulator -destination 'platform=iOS Simulator,name=iPhone 14' test
run: cd ios && set -o pipefail && xcodebuild -workspace sample.xcworkspace -scheme "sample" -sdk iphonesimulator -destination 'platform=iOS Simulator,OS=17.5,name=iPhone 15' -parallel-testing-enabled NO -parallel-testing-worker-count 1 -only-testing:sampleTests - test | bundle exec xcpretty && exit ${PIPESTATUS[0]}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify the scheme includes sampleTests target.

The test command correctly targets the sampleTests bundle and disables parallel testing for reproducibility. However, verify that the shared sample.xcscheme includes a TestableReference for sampleTests so xcodebuild actually discovers and runs the tests.

Based on past review comments, run this script to confirm the scheme configuration:


🏁 Script executed:

#!/bin/bash
# Verify sampleTests is referenced in the shared scheme
if [ -f "sample/ios/sample.xcodeproj/xcshareddata/xcschemes/sample.xcscheme" ]; then
  echo "Checking sample.xcscheme for sampleTests reference:"
  if grep -q 'BlueprintName="sampleTests"' sample/ios/sample.xcodeproj/xcshareddata/xcschemes/sample.xcscheme; then
    echo "✓ sampleTests is referenced in the scheme"
  else
    echo "✗ WARNING: sampleTests is NOT referenced in the scheme"
    echo "Tests may not run. Add a TestableReference entry under <TestAction>."
  fi
else
  echo "✗ Shared scheme not found"
fi

Length of output: 175


Add sampleTests to shared Xcode scheme
In sample/ios/sample.xcodeproj/xcshareddata/xcschemes/sample.xcscheme, add a <TestableReference BlueprintName="sampleTests" ...> entry under the <TestAction> element; without it xcodebuild won’t discover or run your tests.

🤖 Prompt for AI Agents
In .github/workflows/ci.yml lines 80-82, the workflow runs xcodebuild but the
shared Xcode scheme doesn't list the test target, so xcodebuild can't discover
or run tests; open
sample/ios/sample.xcodeproj/xcshareddata/xcschemes/sample.xcscheme, locate the
<TestAction> element and add a <TestableReference> entry for the test target
(e.g. <TestableReference BlueprintName="sampleTests"
BlueprintIdentifier="REPLACE_WITH_TEST_TARGET_GUID"
ContainerRelativePath="sample.xcodeproj">...</TestableReference>), ensuring
BlueprintName matches your test target name and
BlueprintIdentifier/ContainerRelativePath point to the correct test
target/project; save the updated shared scheme so CI's xcodebuild will discover
and run sampleTests.

@islameldesoky95 islameldesoky95 force-pushed the chore/MSDK-2613-fix-ios-sample-tests branch from 7ba3f91 to 1f1607a Compare October 9, 2025 14:13
@islameldesoky95 islameldesoky95 force-pushed the chore/MSDK-2613-fix-ios-sample-tests branch from 1f1607a to e513cf2 Compare October 9, 2025 14:20
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7ba3f91 and e513cf2.

⛔ Files ignored due to path filters (1)
  • sample/ios/Podfile.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • .github/workflows/ci.yml (3 hunks)
  • sample/Gemfile (1 hunks)
🧰 Additional context used
🪛 RuboCop (1.81.1)
sample/Gemfile

[convention] 9-9: Gems should be sorted in an alphabetical order within their section of the Gemfile. Gem activesupport should appear before xcodeproj.

(Bundler/OrderedGems)


[convention] 15-15: Gems should be sorted in an alphabetical order within their section of the Gemfile. Gem benchmark should appear before logger.

(Bundler/OrderedGems)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test-android
  • GitHub Check: test-ios
🔇 Additional comments (1)
sample/Gemfile (1)

7-7: No changes required for cocoapods version
RubyGems confirms that cocoapods 1.16.2 is published, so the current constraint is valid.

@Usercentrics Usercentrics deleted a comment from coderabbitai bot Oct 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants