From b64e143ae0f6b91eb6a78d46899a2d3a6f9e2225 Mon Sep 17 00:00:00 2001 From: Ben White Date: Thu, 5 Dec 2024 17:25:45 +0100 Subject: [PATCH] Fix tests --- src/__tests__/consent.test.ts | 2 ++ src/__tests__/decide.ts | 5 ++- src/__tests__/extensions/web-vitals.test.ts | 3 +- src/__tests__/featureflags.ts | 6 ++-- src/__tests__/heatmaps.test.ts | 3 +- src/__tests__/helpers/mock-logger.ts | 32 +++++++++++++++++++ src/__tests__/identify.test.ts | 12 +++---- src/__tests__/personProcessing.test.ts | 27 ++++++++-------- src/__tests__/posthog-core.beforeSend.test.ts | 13 +++----- src/__tests__/posthog-core.ts | 21 ++++-------- src/__tests__/rate-limiter.test.ts | 8 ++--- src/__tests__/site-apps.ts | 13 +++----- src/__tests__/utils/number-utils.test.ts | 11 ++----- 13 files changed, 87 insertions(+), 69 deletions(-) create mode 100644 src/__tests__/helpers/mock-logger.ts diff --git a/src/__tests__/consent.test.ts b/src/__tests__/consent.test.ts index b6f520ee5..7eccd199f 100644 --- a/src/__tests__/consent.test.ts +++ b/src/__tests__/consent.test.ts @@ -1,3 +1,5 @@ +import './helpers/mock-logger' + import { PostHog } from '../posthog-core' import { defaultPostHog } from './helpers/posthog-instance' import { uuidv7 } from '../uuidv7' diff --git a/src/__tests__/decide.ts b/src/__tests__/decide.ts index b770283ac..a7c8ec266 100644 --- a/src/__tests__/decide.ts +++ b/src/__tests__/decide.ts @@ -212,7 +212,10 @@ describe('Decide', () => { subject(undefined as unknown as DecideResponse) expect(posthog.featureFlags.receivedFeatureFlags).toHaveBeenCalledWith({}, true) - expect(console.error).toHaveBeenCalledWith('[PostHog.js]', 'Failed to fetch feature flags from PostHog.') + expect(console.error).toHaveBeenCalledWith( + '[PostHog.js] [Decide]', + 'Failed to fetch feature flags from PostHog.' + ) }) it('Make sure receivedFeatureFlags is not called if advanced_disable_feature_flags_on_first_load is set', () => { diff --git a/src/__tests__/extensions/web-vitals.test.ts b/src/__tests__/extensions/web-vitals.test.ts index b28aeb125..0aa4450fc 100644 --- a/src/__tests__/extensions/web-vitals.test.ts +++ b/src/__tests__/extensions/web-vitals.test.ts @@ -1,3 +1,5 @@ +import '../helpers/mock-logger' + import { createPosthogInstance } from '../helpers/posthog-instance' import { uuidv7 } from '../../uuidv7' import { PostHog } from '../../posthog-core' @@ -5,7 +7,6 @@ import { DecideResponse, PerformanceCaptureConfig, SupportedWebVitalsMetrics } f import { assignableWindow } from '../../utils/globals' import { DEFAULT_FLUSH_TO_CAPTURE_TIMEOUT_MILLISECONDS, FIFTEEN_MINUTES_IN_MILLIS } from '../../extensions/web-vitals' -jest.mock('../../utils/logger') jest.useFakeTimers() describe('web vitals', () => { diff --git a/src/__tests__/featureflags.ts b/src/__tests__/featureflags.ts index 7dd295c6c..8dd585b94 100644 --- a/src/__tests__/featureflags.ts +++ b/src/__tests__/featureflags.ts @@ -87,7 +87,7 @@ describe('featureflags', () => { expect(featureFlags.getFlags()).toEqual([]) expect(featureFlags.isFeatureEnabled('beta-feature')).toEqual(undefined) expect(window.console.warn).toHaveBeenCalledWith( - '[PostHog.js]', + '[PostHog.js] [FeatureFlags]', 'isFeatureEnabled for key "beta-feature" failed. Feature flags didn\'t load in time.' ) @@ -95,7 +95,7 @@ describe('featureflags', () => { expect(featureFlags.getFeatureFlag('beta-feature')).toEqual(undefined) expect(window.console.warn).toHaveBeenCalledWith( - '[PostHog.js]', + '[PostHog.js] [FeatureFlags]', 'getFeatureFlag for key "beta-feature" failed. Feature flags didn\'t load in time.' ) }) @@ -246,7 +246,7 @@ describe('featureflags', () => { 'alpha-feature-2': true, }) expect(window.console.warn).toHaveBeenCalledWith( - '[PostHog.js]', + '[PostHog.js] [FeatureFlags]', ' Overriding feature flags!', expect.any(Object) ) diff --git a/src/__tests__/heatmaps.test.ts b/src/__tests__/heatmaps.test.ts index 5168c8a37..a346cd476 100644 --- a/src/__tests__/heatmaps.test.ts +++ b/src/__tests__/heatmaps.test.ts @@ -1,3 +1,5 @@ +import './helpers/mock-logger' + import { createPosthogInstance } from './helpers/posthog-instance' import { uuidv7 } from '../uuidv7' import { PostHog } from '../posthog-core' @@ -7,7 +9,6 @@ import { beforeEach, expect } from '@jest/globals' import { HEATMAPS_ENABLED_SERVER_SIDE } from '../constants' import { Heatmaps } from '../heatmaps' -jest.mock('../utils/logger') jest.useFakeTimers() describe('heatmaps', () => { diff --git a/src/__tests__/helpers/mock-logger.ts b/src/__tests__/helpers/mock-logger.ts new file mode 100644 index 000000000..b230cde9e --- /dev/null +++ b/src/__tests__/helpers/mock-logger.ts @@ -0,0 +1,32 @@ +import { Logger } from '../../utils/logger' + +jest.mock('../../utils/logger', () => { + const mockLogger: Logger = { + _log: jest.fn(), + critical: jest.fn(), + uninitializedWarning: jest.fn(), + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + createLogger: () => { + return mockLogger + }, + } + return { + logger: mockLogger, + createLogger: mockLogger.createLogger, + } +}) + +import { logger } from '../../utils/logger' +import { isFunction } from '../../utils/type-utils' + +export const clearLoggerMocks = () => { + Object.values(logger).forEach((mock: any) => { + if (isFunction(mock.mockClear)) { + mock.mockClear() + } + }) +} + +export const mockLogger: jest.Mocked = logger as any diff --git a/src/__tests__/identify.test.ts b/src/__tests__/identify.test.ts index d9f37571d..6c9e6139d 100644 --- a/src/__tests__/identify.test.ts +++ b/src/__tests__/identify.test.ts @@ -1,7 +1,7 @@ +import { mockLogger } from './helpers/mock-logger' + import { createPosthogInstance } from './helpers/posthog-instance' -import { logger } from '../utils/logger' import { uuidv7 } from '../uuidv7' -jest.mock('../utils/logger') describe('identify', () => { // Note that there are other tests for identify in posthog-core.identify.js @@ -19,8 +19,8 @@ describe('identify', () => { // assert expect(posthog.persistence!.properties()['$user_id']).toEqual(distinctId) - expect(jest.mocked(logger).error).toBeCalledTimes(0) - expect(jest.mocked(logger).warn).toBeCalledTimes(0) + expect(mockLogger.error).toBeCalledTimes(0) + expect(mockLogger.warn).toBeCalledTimes(0) }) it('should convert a numeric distinct_id to a string', async () => { @@ -35,8 +35,8 @@ describe('identify', () => { // assert expect(posthog.persistence!.properties()['$user_id']).toEqual(distinctIdString) - expect(jest.mocked(logger).error).toBeCalledTimes(0) - expect(jest.mocked(logger).warn).toBeCalledTimes(1) + expect(mockLogger.error).toBeCalledTimes(0) + expect(mockLogger.warn).toBeCalledTimes(1) }) it('should send $is_identified = true with the identify event and following events', async () => { diff --git a/src/__tests__/personProcessing.test.ts b/src/__tests__/personProcessing.test.ts index 12abe13b3..058dedb42 100644 --- a/src/__tests__/personProcessing.test.ts +++ b/src/__tests__/personProcessing.test.ts @@ -1,11 +1,10 @@ +import { mockLogger } from './helpers/mock-logger' + import { createPosthogInstance } from './helpers/posthog-instance' import { uuidv7 } from '../uuidv7' -import { logger } from '../utils/logger' import { INITIAL_CAMPAIGN_PARAMS, INITIAL_REFERRER_INFO } from '../constants' import { RemoteConfig } from '../types' -jest.mock('../utils/logger') - const INITIAL_CAMPAIGN_PARAMS_NULL = { $initial_current_url: null, $initial_dclid: null, @@ -156,8 +155,8 @@ describe('person processing', () => { posthog.identify(distinctId) // assert - expect(jest.mocked(logger).error).toBeCalledTimes(1) - expect(jest.mocked(logger).error).toHaveBeenCalledWith( + expect(mockLogger.error).toBeCalledTimes(1) + expect(mockLogger.error).toHaveBeenCalledWith( 'posthog.identify was called, but process_person is set to "never". This call will be ignored.' ) expect(beforeSendMock).toBeCalledTimes(0) @@ -172,7 +171,7 @@ describe('person processing', () => { posthog.identify(distinctId) posthog.capture('custom event after identify') // assert - expect(jest.mocked(logger).error).toBeCalledTimes(0) + expect(mockLogger.error).toBeCalledTimes(0) const eventBeforeIdentify = beforeSendMock.mock.calls[0] expect(eventBeforeIdentify[0].properties.$process_person_profile).toEqual(false) const identifyCall = beforeSendMock.mock.calls[1] @@ -191,7 +190,7 @@ describe('person processing', () => { posthog.identify(distinctId) posthog.capture('custom event after identify') // assert - expect(jest.mocked(logger).error).toBeCalledTimes(0) + expect(mockLogger.error).toBeCalledTimes(0) const eventBeforeIdentify = beforeSendMock.mock.calls[0] expect(eventBeforeIdentify[0].properties.$process_person_profile).toEqual(true) const identifyCall = beforeSendMock.mock.calls[1] @@ -503,8 +502,8 @@ describe('person processing', () => { posthog.capture('custom event after group') // assert - expect(jest.mocked(logger).error).toBeCalledTimes(1) - expect(jest.mocked(logger).error).toHaveBeenCalledWith( + expect(mockLogger.error).toBeCalledTimes(1) + expect(mockLogger.error).toHaveBeenCalledWith( 'posthog.group was called, but process_person is set to "never". This call will be ignored.' ) @@ -526,8 +525,8 @@ describe('person processing', () => { // assert expect(beforeSendMock).toBeCalledTimes(0) - expect(jest.mocked(logger).error).toBeCalledTimes(1) - expect(jest.mocked(logger).error).toHaveBeenCalledWith( + expect(mockLogger.error).toBeCalledTimes(1) + expect(mockLogger.error).toHaveBeenCalledWith( 'posthog.setPersonProperties was called, but process_person is set to "never". This call will be ignored.' ) }) @@ -593,8 +592,8 @@ describe('person processing', () => { // assert expect(beforeSendMock).toBeCalledTimes(0) - expect(jest.mocked(logger).error).toBeCalledTimes(1) - expect(jest.mocked(logger).error).toHaveBeenCalledWith( + expect(mockLogger.error).toBeCalledTimes(1) + expect(mockLogger.error).toHaveBeenCalledWith( 'posthog.alias was called, but process_person is set to "never". This call will be ignored.' ) }) @@ -644,7 +643,7 @@ describe('person processing', () => { // assert expect(beforeSendMock).toBeCalledTimes(0) - expect(jest.mocked(logger).error).toBeCalledTimes(0) + expect(mockLogger.error).toBeCalledTimes(0) }) }) diff --git a/src/__tests__/posthog-core.beforeSend.test.ts b/src/__tests__/posthog-core.beforeSend.test.ts index 35d0a4641..d4f1c0322 100644 --- a/src/__tests__/posthog-core.beforeSend.test.ts +++ b/src/__tests__/posthog-core.beforeSend.test.ts @@ -1,11 +1,10 @@ +import { mockLogger } from './helpers/mock-logger' + import { uuidv7 } from '../uuidv7' import { defaultPostHog } from './helpers/posthog-instance' -import { logger } from '../utils/logger' import { CaptureResult, knownUnsafeEditableEvent, PostHogConfig } from '../types' import { PostHog } from '../posthog-core' -jest.mock('../utils/logger') - const rejectingEventFn = () => { return null } @@ -53,9 +52,7 @@ describe('posthog core - before send', () => { expect(capturedData).toBeUndefined() expect(posthog._send_request).not.toHaveBeenCalled() - expect(jest.mocked(logger).info).toHaveBeenCalledWith( - `Event '${eventName}' was rejected in beforeSend function` - ) + expect(mockLogger.info).toHaveBeenCalledWith(`Event '${eventName}' was rejected in beforeSend function`) }) it('can edit an event', () => { @@ -156,7 +153,7 @@ describe('posthog core - before send', () => { method: 'POST', url: 'https://us.i.posthog.com/e/', }) - expect(jest.mocked(logger).warn).toHaveBeenCalledWith( + expect(mockLogger.warn).toHaveBeenCalledWith( `Event '${eventName}' has no properties after beforeSend function, this is likely an error.` ) }) @@ -172,7 +169,7 @@ describe('posthog core - before send', () => { posthog.capture(randomUnsafeEditableEvent, {}, {}) - expect(jest.mocked(logger).warn).toHaveBeenCalledWith( + expect(mockLogger.warn).toHaveBeenCalledWith( `Event '${randomUnsafeEditableEvent}' was rejected in beforeSend function. This can cause unexpected behavior.` ) }) diff --git a/src/__tests__/posthog-core.ts b/src/__tests__/posthog-core.ts index 25026ef0a..0579706b4 100644 --- a/src/__tests__/posthog-core.ts +++ b/src/__tests__/posthog-core.ts @@ -1,10 +1,11 @@ +import { mockLogger } from './helpers/mock-logger' + import { Info } from '../utils/event-utils' import { document, window } from '../utils/globals' import { uuidv7 } from '../uuidv7' import * as globals from '../utils/globals' import { ENABLE_PERSON_PROCESSING, USER_STATE } from '../constants' import { createPosthogInstance, defaultPostHog } from './helpers/posthog-instance' -import { logger } from '../utils/logger' import { PostHogConfig, RemoteConfig } from '../types' import { PostHog } from '../posthog-core' import { PostHogPersistence } from '../posthog-persistence' @@ -119,16 +120,14 @@ describe('posthog core', () => { const posthog = posthogWith(defaultConfig, defaultOverrides) posthog._addCaptureHook(hook) - jest.spyOn(logger, 'error').mockImplementation() expect(() => posthog.capture(undefined)).not.toThrow() expect(hook).not.toHaveBeenCalled() - expect(logger.error).toHaveBeenCalledWith('No event name provided to posthog.capture') + expect(mockLogger.error).toHaveBeenCalledWith('No event name provided to posthog.capture') }) it('errors with object event name', () => { const hook = jest.fn() - jest.spyOn(logger, 'error').mockImplementation() const posthog = posthogWith(defaultConfig, defaultOverrides) posthog._addCaptureHook(hook) @@ -136,7 +135,7 @@ describe('posthog core', () => { // @ts-expect-error - testing invalid input expect(() => posthog.capture({ event: 'object as name' })).not.toThrow() expect(hook).not.toHaveBeenCalled() - expect(logger.error).toHaveBeenCalledWith('No event name provided to posthog.capture') + expect(mockLogger.error).toHaveBeenCalledWith('No event name provided to posthog.capture') }) it('respects opt_out_useragent_filter (default: false)', () => { @@ -746,8 +745,6 @@ describe('posthog core', () => { }) it('does nothing when empty', () => { - jest.spyOn(logger, 'warn').mockImplementation() - const posthog = posthogWith({ bootstrap: {}, persistence: 'memory', @@ -756,7 +753,7 @@ describe('posthog core', () => { expect(posthog.get_distinct_id()).not.toBe('abcd') expect(posthog.get_distinct_id()).not.toEqual(undefined) expect(posthog.getFeatureFlag('multivariant')).toBe(undefined) - expect(logger.warn).toHaveBeenCalledWith( + expect(mockLogger.warn).toHaveBeenCalledWith( expect.stringContaining('getFeatureFlag for key "multivariant" failed') ) expect(posthog.getFeatureFlag('disabled')).toBe(undefined) @@ -902,11 +899,9 @@ describe('posthog core', () => { describe('skipped init()', () => { it('capture() does not throw', () => { - jest.spyOn(logger, 'error').mockImplementation() - expect(() => defaultPostHog().capture('$pageview')).not.toThrow() - expect(logger.error).toHaveBeenCalledWith('You must initialize PostHog before calling posthog.capture') + expect(mockLogger.uninitializedWarning).toHaveBeenCalledWith('posthog.capture') }) }) @@ -1112,8 +1107,6 @@ describe('posthog core', () => { }) it('handles loaded config option throwing gracefully', () => { - jest.spyOn(logger, 'critical').mockImplementation() - const posthog = posthogWith( { loaded: () => { @@ -1133,7 +1126,7 @@ describe('posthog core', () => { posthog._loaded() - expect(logger.critical).toHaveBeenCalledWith('`loaded` function failed', expect.anything()) + expect(mockLogger.critical).toHaveBeenCalledWith('`loaded` function failed', expect.anything()) }) describe('/decide', () => { diff --git a/src/__tests__/rate-limiter.test.ts b/src/__tests__/rate-limiter.test.ts index 67455478d..81fcb96c9 100644 --- a/src/__tests__/rate-limiter.test.ts +++ b/src/__tests__/rate-limiter.test.ts @@ -1,8 +1,6 @@ +import { clearLoggerMocks, mockLogger } from './helpers/mock-logger' import { window } from '../../src/utils/globals' import { RateLimiter } from '../rate-limiter' -import { logger } from '../utils/logger' - -jest.mock('../../src/utils/logger') describe('Rate Limiter', () => { let rateLimiter: RateLimiter @@ -44,6 +42,8 @@ describe('Rate Limiter', () => { } rateLimiter = new RateLimiter(mockPostHog as any) + + clearLoggerMocks() }) describe('client side', () => { @@ -270,7 +270,7 @@ describe('Rate Limiter', () => { text: '', }) - expect(jest.mocked(logger).error).not.toHaveBeenCalled() + expect(mockLogger.error).not.toHaveBeenCalled() }) }) }) diff --git a/src/__tests__/site-apps.ts b/src/__tests__/site-apps.ts index aea6407a5..a1f1d6bd1 100644 --- a/src/__tests__/site-apps.ts +++ b/src/__tests__/site-apps.ts @@ -1,19 +1,14 @@ +import { mockLogger } from './helpers/mock-logger' + import { SiteApps } from '../site-apps' import { PostHogPersistence } from '../posthog-persistence' import { RequestRouter } from '../utils/request-router' import { PostHog } from '../posthog-core' import { DecideResponse, PostHogConfig, Properties, CaptureResult } from '../types' import { assignableWindow } from '../utils/globals' -import { logger } from '../utils/logger' import '../entrypoints/external-scripts-loader' import { isFunction } from '../utils/type-utils' -jest.mock('../utils/logger', () => ({ - logger: { - error: jest.fn(), - }, -})) - describe('SiteApps', () => { let posthog: PostHog let siteAppsInstance: SiteApps @@ -314,8 +309,8 @@ describe('SiteApps', () => { siteAppsInstance.onRemoteConfig(response) - expect(logger.error).toHaveBeenCalledWith( - 'PostHog site apps are disabled. Enable the "opt_in_site_apps" config to proceed.' + expect(mockLogger.error).toHaveBeenCalledWith( + 'Site apps exist but "opt_in_site_apps" is not set so they are not loaded.' ) expect(siteAppsInstance.loaded).toBe(true) }) diff --git a/src/__tests__/utils/number-utils.test.ts b/src/__tests__/utils/number-utils.test.ts index 0c6c6cd20..872105a44 100644 --- a/src/__tests__/utils/number-utils.test.ts +++ b/src/__tests__/utils/number-utils.test.ts @@ -1,11 +1,6 @@ -import { clampToRange } from '../../utils/number-utils' -import { logger } from '../../utils/logger' +import { mockLogger } from '../helpers/mock-logger' -jest.mock('../../utils/logger', () => ({ - logger: { - warn: jest.fn(), - }, -})) +import { clampToRange } from '../../utils/number-utils' describe('number-utils', () => { describe('clampToRange', () => { @@ -87,7 +82,7 @@ describe('number-utils', () => { it('logs a warning when min is greater than max', () => { expect(clampToRange(50, 100, 10, 'Test Label')).toBe(10) - expect(logger.warn).toHaveBeenCalledWith('min cannot be greater than max.') + expect(mockLogger.warn).toHaveBeenCalledWith('min cannot be greater than max.') }) }) })