test(analytics-react-native): native iOS XCTest coverage for connectivity module (SDKRN-5) [2.1/5]#1823
Open
Mercy811 wants to merge 7 commits into
Open
test(analytics-react-native): native iOS XCTest coverage for connectivity module (SDKRN-5) [2.1/5]#1823Mercy811 wants to merge 7 commits into
Mercy811 wants to merge 7 commits into
Conversation
Contributor
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b5b433dcd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
size-limit report 📦
|
3826780 to
b52af25
Compare
crleona
approved these changes
Jun 22, 2026
…ectivity module (SDKRN-5) Test source lives in packages/analytics-react-native/ios/Tests (excluded from the shipped pod via exclude_files); the example app's appTests target compiles it as host/runner, replacing the stale RN template test. rn-smoke runs xcodebuild test after the iOS build on both arch legs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
xcodebuild test leaves its destination simulator booted, so the unconditional simctl boot that follows would fail with "Unable to boot device in current state: Booted". bootstatus -b boots only if needed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Use lock.withLock {} instead of manual lock()/defer unlock() in the
EventCapturingConnectivity test helper.
- Reference the test file group-relative ("<group>") instead of
SOURCE_ROOT in the example app's Xcode project.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
b52af25 to
ece5968
Compare
The appTests XCTest target imports the module as a separate module, so
the internal `init()` was invisible and EventCapturingConnectivity could
not be constructed ("no accessible initializers"). Mark it public, like
the other members exposed for the test subclass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
getNetworkConnectivityStatus seeds {isConnected: false} (so startup
events buffer until the monitor reports real status). The test still
asserted the old optimistic-connected seed; update it to expect
disconnected and refresh the stale doc comment on the path-update test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Part of SDKRN-23 (iOS native connectivity module).
PR 2.1 of 5 in the SDKRN-5 offline-mode stack. Stacked on #1819 (base branch
sdkrn-5-offline-2-ios).Adds native iOS XCTest coverage for
AmplitudeReactNativeConnectivity— the follow-up promised in #1819. The key behavior under test:NWPathMonitordelivers the current path as its first update once monitoring starts, the contract the optimistic{isConnected: true}seed ingetNetworkConnectivityStatusrelies on.Where the tests live (and why)
The test source lives in the SDK package —
packages/analytics-react-native/ios/Tests/AmplitudeConnectivityTests.swift— but is compiled and run by the example app's existingappTestsXCTest target, which acts purely as host/runner. A standalone run inside the package isn't possible: the module subclassesRCTEventEmitter, so the tests need React-Core compiled/linked, and RN library pods can't build outside an RN app's pod ecosystem (this is whycreate-react-native-libraryruns native tests via the example app too).s.exclude_files = "ios/Tests/**"keeps the XCTest sources out of the shipped pod (verified: absent from the generated Pods project).Changes
startObserving()→ initialAmplitudeNetworkConnectivityChangedwithisConnected == true(realNWPathMonitor); no events afterstopObserving();supportedEvents. AnEventCapturingConnectivitysubclass overridessendEvent(withName:body:)to capture events without a liveRCTBridge.class→open class; overridden members markedopen/publicso the test target can subclass/call them (Swift also requires the staticrequiresMainQueueSetupoverride to bepubliconce the class is open).appTests.m("Welcome to React", never rendered by this app, Metro-dependent) deleted; the SDK test file wired intoappTestsby file reference;SWIFT_VERSION = 5.0on theappTestsconfigs.Podfile.lockrefreshed (podspec checksum + stale 1.5.54 → 1.5.56 version catch-up).rn-smoke.ymlrunsxcodebuild testafter the iOS build on both arch legs, reusing the same-derivedDataPathso pod compilation is shared; Release config so the host app uses the embedded JS bundle (no Metro).main[5/5]Test plan
xcodebuild test(Release, iPhone 16 simulator): all 4 tests pass.monitor.start(queue:)commented out,testEmitsInitialPathUpdateAfterStartObservingfails at its 10s timeout — the suite genuinely exercises the monitor contract.pnpm --filter @amplitude/analytics-react-native test: jest suite unaffected (108 tests / 11 suites pass).new-archlegs.🤖 Generated with Claude Code