-
Notifications
You must be signed in to change notification settings - Fork 181
feat: improve mixpanel flush strategy #960
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 7 commits
900e655
6775a91
9d70a49
95ee4db
7aa3a7e
af94161
007321b
f6cb0ef
8fd2de0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -8,6 +8,11 @@ require('react-native-gesture-handler/jestSetup'); | |||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| jest.mock('react-native/Libraries/Animated/NativeAnimatedHelper'); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| jest.mock('@env', () => ({ | ||||||||||||||||||||||||||||||||||||||
| ENABLE_DEBUG_LOGS: 'false', | ||||||||||||||||||||||||||||||||||||||
| MIXPANEL_NFC_PROJECT_TOKEN: 'test-token', | ||||||||||||||||||||||||||||||||||||||
| })); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| global.FileReader = class { | ||||||||||||||||||||||||||||||||||||||
| constructor() { | ||||||||||||||||||||||||||||||||||||||
| this.onload = null; | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -197,13 +202,30 @@ jest.mock('react-native-nfc-manager', () => ({ | |||||||||||||||||||||||||||||||||||||
| // Mock react-native-passport-reader | ||||||||||||||||||||||||||||||||||||||
| jest.mock('react-native-passport-reader', () => ({ | ||||||||||||||||||||||||||||||||||||||
| default: { | ||||||||||||||||||||||||||||||||||||||
| initialize: jest.fn(), | ||||||||||||||||||||||||||||||||||||||
| configure: jest.fn(), | ||||||||||||||||||||||||||||||||||||||
| scanPassport: jest.fn(), | ||||||||||||||||||||||||||||||||||||||
| readPassport: jest.fn(), | ||||||||||||||||||||||||||||||||||||||
| cancelPassportRead: jest.fn(), | ||||||||||||||||||||||||||||||||||||||
| trackEvent: jest.fn(), | ||||||||||||||||||||||||||||||||||||||
| flush: jest.fn(), | ||||||||||||||||||||||||||||||||||||||
| reset: jest.fn(), | ||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||
| })); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| const { NativeModules } = require('react-native'); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| NativeModules.PassportReader = { | ||||||||||||||||||||||||||||||||||||||
| configure: jest.fn(), | ||||||||||||||||||||||||||||||||||||||
| scanPassport: jest.fn(), | ||||||||||||||||||||||||||||||||||||||
| trackEvent: jest.fn(), | ||||||||||||||||||||||||||||||||||||||
| flush: jest.fn(), | ||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+220
to
+228
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Keep NativeModules mock consistent and complete
Apply this diff: const { NativeModules } = require('react-native');
NativeModules.PassportReader = {
configure: jest.fn(),
- scanPassport: jest.fn(),
+ scanPassport: jest.fn(),
+ // Alias for parity with d.ts and default export
+ scan: jest.fn(),
trackEvent: jest.fn(),
flush: jest.fn(),
};📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
| jest.mock('@react-native-community/netinfo', () => ({ | ||||||||||||||||||||||||||||||||||||||
| addEventListener: jest.fn(() => jest.fn()), | ||||||||||||||||||||||||||||||||||||||
| fetch: jest.fn(() => Promise.resolve({ isConnected: true })), | ||||||||||||||||||||||||||||||||||||||
| })); | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+229
to
+232
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Duplicate NetInfo mock overrides earlier one (breaks This second Apply this diff to remove the duplicate: -jest.mock('@react-native-community/netinfo', () => ({
- addEventListener: jest.fn(() => jest.fn()),
- fetch: jest.fn(() => Promise.resolve({ isConnected: true })),
-}));Then, update the first NetInfo mock (Lines 147–160) to return an unsubscribe from // In the first mock block (existing):
addEventListener: jest.fn(() => jest.fn()), // returns unsubscribe
fetch: jest.fn().mockResolvedValue({ isConnected: true, isInternetReachable: true }),If you want, I can push a combined single mock that covers 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| // Mock @stablelib packages | ||||||||||||||||||||||||||||||||||||||
| jest.mock('@stablelib/cbor', () => ({ | ||||||||||||||||||||||||||||||||||||||
| encode: jest.fn(), | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| // SPDX-FileCopyrightText: 2025 Social Connect Labs, Inc. | ||
| // SPDX-License-Identifier: BUSL-1.1 | ||
| // NOTE: Converts to Apache-2.0 on 2029-06-11 per LICENSE. | ||
|
|
||
| // Web mock for react-native-passport-reader | ||
| export const reset = async () => { | ||
| // No-op for web | ||
| return Promise.resolve(); | ||
| }; | ||
|
|
||
| export const scan = async () => { | ||
| throw new Error('NFC scanning is not supported on web'); | ||
| }; | ||
transphorm marked this conversation as resolved.
Show resolved
Hide resolved
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Mock parity with types: add
scanalias to match d.ts and potential callsitesThe default export for
react-native-passport-readeronly exposesscanPassport, while the d.ts (and likely some callsites) referencescan. This will cause test failures or undefined calls in environments that import the default module instead ofNativeModules.Apply this diff to add a
scanalias:jest.mock('react-native-passport-reader', () => ({ default: { configure: jest.fn(), - scanPassport: jest.fn(), + scanPassport: jest.fn(), + // Alias to align with TS types/callsites + scan: jest.fn(), readPassport: jest.fn(), cancelPassportRead: jest.fn(), trackEvent: jest.fn(), flush: jest.fn(), reset: jest.fn(), }, }));Run to spot any lingering
scanPassport(vsscan(mismatches:🏁 Script executed:
Length of output: 228
🏁 Script executed:
Length of output: 1200
Add
scanalias to the PassportReader mock to cover Android callsitesYour Android scanner helper calls
PassportReader.scan(…)(app/src/utils/nfcScanner.ts line 128), but your Jest setup only mocksscanPassport. Without ascanmock, tests hittingscanAndroidwill fail withundefined is not a function.Apply this diff in app/jest.setup.js (around lines 203–213):
jest.mock('react-native-passport-reader', () => ({ default: { configure: jest.fn(), scanPassport: jest.fn(), + // Alias to align with TS types/callsites (e.g. scanAndroid) + scan: jest.fn(), readPassport: jest.fn(), cancelPassportRead: jest.fn(), trackEvent: jest.fn(), flush: jest.fn(), reset: jest.fn(), }, }));— without this, any
PassportReader.scan(...)calls will be unmocked and break your Android tests.📝 Committable suggestion
🤖 Prompt for AI Agents