From 3ef3a01103a96759a18ac0c1c17b2e983a034dff Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Thu, 5 Sep 2024 20:11:19 +0600 Subject: [PATCH 1/7] [FSSDK-10616] experiment test improvement --- src/Experiment.spec.tsx | 136 ++++++++++++++++++++-------------------- 1 file changed, 68 insertions(+), 68 deletions(-) diff --git a/src/Experiment.spec.tsx b/src/Experiment.spec.tsx index bd828cd..97e6951 100644 --- a/src/Experiment.spec.tsx +++ b/src/Experiment.spec.tsx @@ -25,10 +25,14 @@ import { OptimizelyProvider } from './Provider'; import { ReactSDKClient } from './client'; import { OptimizelyVariation } from './Variation'; +type Resolver = { + resolve: (value: { success: boolean; reason?: string }) => void; + reject: (reason?: string) => void; +}; + describe('', () => { const variationKey = 'matchingVariation'; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - let resolver: any; + let resolver: Resolver; let optimizelyMock: ReactSDKClient; let isReady: boolean; @@ -58,17 +62,40 @@ describe('', () => { getIsReadyPromiseFulfilled: () => true, getIsUsingSdkKey: () => true, onForcedVariationsUpdate: jest.fn().mockReturnValue(() => {}), + setUser: jest.fn(), } as unknown as ReactSDKClient; }); it('does not throw an error when not rendered in the context of an OptimizelyProvider', () => { - expect(() => { - render( + const { container } = render( + + {(variation: string | null) => {variation}} + + ); + + expect(container).toBeDefined(); + }); + + it('isValidElement check works as expected', async () => { + const { container } = render( + - {(variation: string) => {variation}} + {(variation: string | null) => ( + <> + {variation} + {null} + {
} + + )} - ); - }).toBeDefined(); + + ); + resolver.resolve({ success: true }); + + await waitFor(() => { + const validChildren = container.getElementsByTagName('span'); + expect(validChildren).toHaveLength(1); + }); }); describe('when isServerSide prop is false', () => { @@ -76,7 +103,7 @@ describe('', () => { const { container, rerender } = render( - {(variation: string) => {variation}} + {(variation: string | null) => {variation}} ); @@ -89,12 +116,10 @@ describe('', () => { // Simulate client becoming ready: onReady resolving, firing config update notification resolver.resolve({ success: true }); - await optimizelyMock.onReady(); - rerender( - {(variation: string) => {variation}} + {(variation: string | null) => {variation}} ); @@ -108,7 +133,7 @@ describe('', () => { const { container } = render( - {(variation: string) => {variation}} + {(variation: string | null) => {variation}} ); @@ -118,18 +143,17 @@ describe('', () => { // while it's waiting for onReady() expect(container.innerHTML).toBe(''); - // Simulate client becoming ready; onReady resolving, firing config update notification + // Simulate client becoming ready; oReady resolving, firing config update notification resolver.resolve({ success: true }); - await optimizelyMock.onReady(); - expect(optimizelyMock.activate).toHaveBeenCalledWith('experiment1', undefined, undefined); + await waitFor(() => expect(optimizelyMock.activate).toHaveBeenCalledWith('experiment1', undefined, undefined)); }); it(`should use the Experiment prop's timeout when there is no timeout passed to `, async () => { const { container } = render( - {(variation: string) => {variation}} + {(variation: string | null) => {variation}} ); @@ -141,9 +165,8 @@ describe('', () => { // Simulate client becoming ready resolver.resolve({ success: true }); - await optimizelyMock.onReady(); - expect(optimizelyMock.activate).toHaveBeenCalledWith('experiment1', undefined, undefined); + await waitFor(() => expect(optimizelyMock.activate).toHaveBeenCalledWith('experiment1', undefined, undefined)); }); it('should render using when the variationKey matches', async () => { @@ -162,14 +185,13 @@ describe('', () => { ); + // while it's waiting for onReady() expect(container.innerHTML).toBe(''); // Simulate client becoming ready resolver.resolve({ success: true }); - await optimizelyMock.onReady(); - await waitFor(() => expect(screen.getByTestId('variation-key')).toHaveTextContent('correct variation')); }); @@ -194,8 +216,6 @@ describe('', () => { // Simulate client becoming ready resolver.resolve({ success: true }); - await optimizelyMock.onReady(); - await waitFor(() => expect(screen.getByTestId('variation-key')).toHaveTextContent('default variation')); }); @@ -219,8 +239,6 @@ describe('', () => { // Simulate client becoming ready resolver.resolve({ success: true }); - await optimizelyMock.onReady(); - await waitFor(() => expect(screen.getByTestId('variation-key')).toHaveTextContent('matching variation')); }); @@ -244,12 +262,10 @@ describe('', () => { // Simulate client becoming ready resolver.resolve({ success: true }); - await optimizelyMock.onReady(); - await waitFor(() => expect(screen.getByTestId('variation-key')).toHaveTextContent('default variation')); }); - describe('a OptimizelyVariation with default & variation props', () => { + describe('OptimizelyVariation with default & variation props', () => { it('should render default with NO matching variations ', async () => { const { container } = render( @@ -270,8 +286,6 @@ describe('', () => { // Simulate client becoming ready resolver.resolve({ success: true }); - await optimizelyMock.onReady(); - await waitFor(() => expect(screen.getByTestId('variation-key')).toHaveTextContent('default & non matching variation') ); @@ -297,8 +311,6 @@ describe('', () => { // Simulate client becoming ready resolver.resolve({ success: true }); - await optimizelyMock.onReady(); - await waitFor(() => expect(screen.getByTestId('variation-key')).toHaveTextContent('matching variation')); }); }); @@ -329,8 +341,6 @@ describe('', () => { // Simulate client becoming ready resolver.resolve({ success: true }); - await optimizelyMock.onReady(); - await waitFor(() => expect(screen.getByTestId('variation-key')).toHaveTextContent('non-matching variation 3')); }); @@ -354,8 +364,6 @@ describe('', () => { // Simulate client becoming ready resolver.resolve({ success: true }); - await optimizelyMock.onReady(); - expect(container.innerHTML).toBe(''); }); @@ -367,7 +375,7 @@ describe('', () => { overrideUserId="james123" overrideAttributes={{ betaUser: true }} > - {(variation: string) => {variation}} + {(variation: string | null) => {variation}} ); @@ -380,18 +388,17 @@ describe('', () => { // Simulate client becoming ready resolver.resolve({ success: true }); - await optimizelyMock.onReady(); - - expect(optimizelyMock.activate).toHaveBeenCalledWith('experiment1', 'james123', { betaUser: true }); - - await waitFor(() => expect(screen.getByTestId('variation-key')).toHaveTextContent('matchingVariation')); + await waitFor(() => { + expect(optimizelyMock.activate).toHaveBeenCalledWith('experiment1', 'james123', { betaUser: true }); + expect(screen.getByTestId('variation-key')).toHaveTextContent('matchingVariation'); + }); }); it('should pass the values for clientReady and didTimeout', async () => { const { container } = render( - {(variation: string, clientReady: boolean, didTimeout: boolean) => ( + {(variation: string | null, clientReady?: boolean, didTimeout?: boolean) => ( {`${variation}|${clientReady}|${didTimeout}`} )} @@ -404,12 +411,10 @@ describe('', () => { // Simulate client becoming ready resolver.resolve({ success: true }); - await optimizelyMock.onReady(); - - expect(optimizelyMock.activate).toHaveBeenCalledWith('experiment1', undefined, undefined); - await waitFor(() => - expect(screen.getByTestId('variation-key')).toHaveTextContent('matchingVariation|true|false') - ); + await waitFor(() => { + expect(optimizelyMock.activate).toHaveBeenCalledWith('experiment1', undefined, undefined); + expect(screen.getByTestId('variation-key')).toHaveTextContent('matchingVariation|true|false'); + }); }); describe('when the onReady() promise return { success: false }', () => { @@ -431,9 +436,8 @@ describe('', () => { expect(container.innerHTML).toBe(''); resolver.resolve({ success: false, reason: 'fail' }); - await optimizelyMock.onReady(); - expect(container.innerHTML).toBe(''); + await waitFor(() => expect(container.innerHTML).toBe('')); }); }); }); @@ -443,9 +447,7 @@ describe('', () => { const { container } = render( - {(variation: string, clientReady: boolean, didTimeout: boolean) => ( - {variation} - )} + {(variation: string | null) => {variation}} ); @@ -457,11 +459,11 @@ describe('', () => { // Simulate client becoming ready resolver.resolve({ success: true }); isReady = true; - await act(async () => await optimizelyMock.onReady()); - - expect(optimizelyMock.activate).toHaveBeenCalledWith('experiment1', undefined, undefined); - await waitFor(() => expect(screen.getByTestId('variation-key')).toHaveTextContent('matchingVariation')); + await waitFor(() => { + expect(optimizelyMock.activate).toHaveBeenCalledWith('experiment1', undefined, undefined); + expect(screen.getByTestId('variation-key')).toHaveTextContent('matchingVariation'); + }); // capture the OPTIMIZELY_CONFIG_UPDATE function // change the return value of activate @@ -473,16 +475,14 @@ describe('', () => { expect(optimizelyMock.activate).toHaveBeenCalledWith('experiment1', undefined, undefined); await waitFor(() => expect(screen.getByTestId('variation-key')).toHaveTextContent('newVariation')); - expect(optimizelyMock.activate).toBeCalledTimes(2); + expect(optimizelyMock.activate).toHaveBeenCalledTimes(2); }); it('should re-render when the user changes', async () => { const { container } = render( - {(variation: string, clientReady: boolean, didTimeout: boolean) => ( - {variation} - )} + {(variation: string | null) => {variation}} ); @@ -493,10 +493,11 @@ describe('', () => { // Simulate client becoming ready resolver.resolve({ success: true }); isReady = true; - await act(async () => await optimizelyMock.onReady()); - expect(optimizelyMock.activate).toHaveBeenCalledWith('experiment1', undefined, undefined); - await waitFor(() => expect(screen.getByTestId('variation-key')).toHaveTextContent('matchingVariation')); + await waitFor(() => { + expect(optimizelyMock.activate).toHaveBeenCalledWith('experiment1', undefined, undefined); + expect(screen.getByTestId('variation-key')).toHaveTextContent('matchingVariation'); + }); // capture the onUserUpdate function const updateFn = (optimizelyMock.onUserUpdate as jest.Mock).mock.calls[0][0]; @@ -506,7 +507,7 @@ describe('', () => { expect(optimizelyMock.activate).toHaveBeenCalledWith('experiment1', undefined, undefined); await waitFor(() => expect(screen.getByTestId('variation-key')).toHaveTextContent('newVariation')); - expect(optimizelyMock.activate).toBeCalledTimes(2); + expect(optimizelyMock.activate).toHaveBeenCalledTimes(2); }); }); @@ -515,9 +516,7 @@ describe('', () => { render( - {(variation: string, clientReady: boolean, didTimeout: boolean) => ( - {variation} - )} + {(variation: string | null) => {variation}} ); @@ -541,6 +540,7 @@ describe('', () => { ); + await waitFor(() => expect(screen.getByTestId('variation-key')).toHaveTextContent('correct variation')); }); }); From c201a23051dd50897ce105a224abad913ed640ff Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Thu, 5 Sep 2024 20:59:25 +0600 Subject: [PATCH 2/7] [FSSDK-10616] logOnlyEventDispatcher improvement --- src/Experiment.spec.tsx | 2 +- src/logOnlyEventDispatcher.spec.ts | 24 +++++++++++++++++++++++- src/logOnlyEventDispatcher.ts | 6 ++---- 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/Experiment.spec.tsx b/src/Experiment.spec.tsx index 97e6951..3686538 100644 --- a/src/Experiment.spec.tsx +++ b/src/Experiment.spec.tsx @@ -143,7 +143,7 @@ describe('', () => { // while it's waiting for onReady() expect(container.innerHTML).toBe(''); - // Simulate client becoming ready; oReady resolving, firing config update notification + // Simulate client becoming ready; onReady resolving, firing config update notification resolver.resolve({ success: true }); await waitFor(() => expect(optimizelyMock.activate).toHaveBeenCalledWith('experiment1', undefined, undefined)); diff --git a/src/logOnlyEventDispatcher.spec.ts b/src/logOnlyEventDispatcher.spec.ts index 7edd53f..c3b1a43 100644 --- a/src/logOnlyEventDispatcher.spec.ts +++ b/src/logOnlyEventDispatcher.spec.ts @@ -24,10 +24,32 @@ import { getLogger } from '@optimizely/optimizely-sdk'; const logger = getLogger('ReactSDK'); describe('logOnlyEventDispatcher', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it('logs a message', () => { const callback = jest.fn(); - logOnlyEventDispatcher.dispatchEvent({ url: 'https://localhost:8080', httpVerb: 'POST', params: {} }, callback); + const mockEvent = { url: 'https://localhost:8080', httpVerb: 'POST' as const, params: {} }; + logOnlyEventDispatcher.dispatchEvent(mockEvent, callback); + const secondArgFunction = (logger.debug as jest.Mock).mock.calls[0][1]; + const result = secondArgFunction(); + expect(callback).toHaveBeenCalled(); expect(logger.debug).toHaveBeenCalled(); + expect(result).toBe(JSON.stringify(mockEvent)); + }); + + it('debugger log print error stringifying event', () => { + const callback = jest.fn(); + // circular reference to force JSON.stringify to throw an error + const circularReference: any = {}; + circularReference.self = circularReference; + logOnlyEventDispatcher.dispatchEvent(circularReference, callback); + const secondArgFunction = (logger.debug as jest.Mock).mock.calls[0][1]; + const result = secondArgFunction(); + + expect(typeof secondArgFunction).toBe('function'); + expect(result).toBe('error stringifying event'); }); }); diff --git a/src/logOnlyEventDispatcher.ts b/src/logOnlyEventDispatcher.ts index 933edf8..89c8e90 100644 --- a/src/logOnlyEventDispatcher.ts +++ b/src/logOnlyEventDispatcher.ts @@ -1,5 +1,5 @@ /** - * Copyright 2019, 2023 Optimizely + * Copyright 2019, 2023-2024 Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,10 +15,8 @@ */ import * as optimizely from '@optimizely/optimizely-sdk'; -import { getLogger } from '@optimizely/optimizely-sdk'; - -const logger = getLogger('ReactSDK'); +const logger = optimizely.getLogger('ReactSDK'); /** * logOnlyEventDispatcher only logs a message at the debug level, and does not * send any requests to the Optimizely results backend. Use this to disable From 2c26938a6eefce1cde86ddc3c84b4c6aa541e999 Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Fri, 6 Sep 2024 17:05:14 +0600 Subject: [PATCH 3/7] [FSSDK-10616] logger improvement --- src/logger.spec.ts | 78 ++++++++++++++++++++++++++++++++++++++++++++++ src/logger.tsx | 2 +- 2 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 src/logger.spec.ts diff --git a/src/logger.spec.ts b/src/logger.spec.ts new file mode 100644 index 0000000..44a22d9 --- /dev/null +++ b/src/logger.spec.ts @@ -0,0 +1,78 @@ +/** + * Copyright 2024 Optimizely + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as optimizely from '@optimizely/optimizely-sdk'; +import { logger } from './logger'; +import { sprintf } from './utils'; + +jest.mock('@optimizely/optimizely-sdk', () => ({ + getLogger: jest.fn().mockReturnValue({ + log: jest.fn(), + }), + enums: { + LOG_LEVEL: { + WARNING: 'WARNING', + INFO: 'INFO', + DEBUG: 'DEBUG', + ERROR: 'ERROR', + }, + }, +})); + +describe('logger module', () => { + const mockLogHandler = optimizely.getLogger('ReactSDK'); + const logSpy = mockLogHandler.log as jest.Mock; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should log a warning message', () => { + const message = 'This is a warning: %s'; + const arg = 'something went wrong'; + + logger.warn(message, arg); + + expect(logSpy).toHaveBeenCalledWith(optimizely.enums.LOG_LEVEL.WARNING, sprintf(message, arg)); + }); + + it('should log an info message', () => { + const message = 'This is an info: %s'; + const arg = 'all good'; + + logger.info(message, arg); + + expect(logSpy).toHaveBeenCalledWith(optimizely.enums.LOG_LEVEL.INFO, sprintf(message, arg)); + }); + + it('should log a debug message', () => { + const message = 'Debugging: %s'; + const arg = 'checking details'; + + logger.debug(message, arg); + + expect(logSpy).toHaveBeenCalledWith(optimizely.enums.LOG_LEVEL.DEBUG, sprintf(message, arg)); + }); + + it('should log an error message', () => { + const message = 'Error occurred: %s'; + const arg = 'critical failure'; + + logger.error(message, arg); + + expect(logSpy).toHaveBeenCalledWith(optimizely.enums.LOG_LEVEL.ERROR, sprintf(message, arg)); + }); +}); diff --git a/src/logger.tsx b/src/logger.tsx index 4c230ab..7703155 100644 --- a/src/logger.tsx +++ b/src/logger.tsx @@ -1,5 +1,5 @@ /** - * Copyright 2022, Optimizely + * Copyright 2022,2024 Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. From 6daf36fff731842f4c4e2b019def5b7fbcb905db Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Fri, 6 Sep 2024 19:31:53 +0600 Subject: [PATCH 4/7] [FSSDK-10616] utils improvement --- src/utils.spec.tsx | 194 +++++++++++++++++++++++++++++++++++++++++++++ src/utils.tsx | 21 ----- 2 files changed, 194 insertions(+), 21 deletions(-) create mode 100644 src/utils.spec.tsx diff --git a/src/utils.spec.tsx b/src/utils.spec.tsx new file mode 100644 index 0000000..07b95c4 --- /dev/null +++ b/src/utils.spec.tsx @@ -0,0 +1,194 @@ +/** + * Copyright 2024 Optimizely + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import * as utils from './utils'; +import React, { forwardRef } from 'react'; +import { render, screen } from '@testing-library/react'; +import hoistNonReactStatics from 'hoist-non-react-statics'; +import { UserInfo } from './utils'; + +describe('utils', () => { + describe('areUsersEqual', () => { + const user = { id: '1', attributes: { name: 'user1' } }; + + it('returns true if users are equal', () => { + const user2 = JSON.parse(JSON.stringify(user)); + + expect(utils.areUsersEqual(user, user2)).toBe(true); + }); + + it('returns false if users are not equal', () => { + const user2 = { id: '2', attributes: { name: 'user2' } }; + + expect(utils.areUsersEqual(user, user2)).toBe(false); + }); + + it('returns false if key lengths are not equal', () => { + const user2 = { id: '1', attributes: { name: 'user1', age: 30 } }; + + expect(utils.areUsersEqual(user, user2)).toBe(false); + }); + + it('returns false if one of the key value pairs are not equal', () => { + const user2 = { id: '1', attributes: { name: 'user2' } }; + + expect(utils.areUsersEqual(user, user2)).toBe(false); + }); + }); + describe('hoistStaticsAndForwardRefs', () => { + class TestComponent extends React.Component<{ forwardedRef?: React.Ref }> { + static testStaticMethod = () => 'static method result'; + + render() { + return ( +
+ Hello +
+ ); + } + } + + // source component with statics, that should be available in the wrapped component + class SourceComponent extends React.Component { + static sourceStaticMethod = () => 'source static method result'; + + render() { + return
; + } + } + + const WrappedComponent = utils.hoistStaticsAndForwardRefs(TestComponent, SourceComponent, 'WrappedComponent'); + + it('should forward refs and hoist static methods', () => { + const ref = React.createRef(); + + render(); + + expect(ref.current).toBeDefined(); + expect(ref.current?.nodeName).toBe('DIV'); + expect(screen.getByTestId('test-div')).toBe(ref.current); + + // @ts-ignore + expect(WrappedComponent.sourceStaticMethod()).toBe('source static method result'); + }); + }); + + describe('areAttributesEqual', () => { + it('should return true for equal attributes', () => { + const attrs1 = { a: 1, b: 2 }; + const attrs2 = { a: 1, b: 2 }; + + expect(utils.areAttributesEqual(attrs1, attrs2)).toBe(true); + }); + + it('should return false for different attribute keys', () => { + const attrs1 = { a: 1, b: 2 }; + const attrs2 = { a: 1, c: 2 }; + + expect(utils.areAttributesEqual(attrs1, attrs2)).toBe(false); + }); + + it('should return false for different attribute values', () => { + const attrs1 = { a: 1, b: 2 }; + const attrs2 = { a: 1, b: 3 }; + + expect(utils.areAttributesEqual(attrs1, attrs2)).toBe(false); + }); + + it('should return false if the number of attributes differs', () => { + const attrs1 = { a: 1, b: 2 }; + const attrs2 = { a: 1 }; + + expect(utils.areAttributesEqual(attrs1, attrs2)).toBe(false); + }); + + it('should handle undefined or null attributes as empty objects', () => { + const attrs1 = null; + const attrs2 = undefined; + + expect(utils.areAttributesEqual(attrs1, attrs2)).toBe(true); + }); + + it('should return false when one attribute is an object and another is not', () => { + const attrs1 = { a: 1 }; + const attrs2 = 'not an object'; + + expect(utils.areAttributesEqual(attrs1, attrs2)).toBe(false); + }); + + it('should handle different types of attribute values correctly', () => { + const attrs1 = { a: '1', b: true }; + const attrs2 = { a: '1', b: true }; + + expect(utils.areAttributesEqual(attrs1, attrs2)).toBe(true); + }); + }); + + describe('createFailedDecision', () => { + it('should return a correctly formatted OptimizelyDecision object', () => { + const flagKey = 'testFlag'; + const message = 'Decision failed due to some reason'; + const user: UserInfo = { + id: 'user123', + attributes: { age: 25, location: 'NY' }, + }; + + const expectedDecision: utils.OptimizelyDecision = { + enabled: false, + flagKey: 'testFlag', + ruleKey: null, + variationKey: null, + variables: {}, + reasons: ['Decision failed due to some reason'], + userContext: { + id: 'user123', + attributes: { age: 25, location: 'NY' }, + }, + }; + + const result = utils.createFailedDecision(flagKey, message, user); + + expect(result).toEqual(expectedDecision); + }); + }); + + describe('sprintf', () => { + it('should replace %s with a string argument', () => { + expect(utils.sprintf('Hello, %s!', 'world')).toBe('Hello, world!'); + }); + + it('should replace %s with a function result', () => { + const dynamicString = () => 'dynamic'; + expect(utils.sprintf('This is a %s string.', dynamicString)).toBe('This is a dynamic string.'); + }); + + it('should replace %s with various types of arguments', () => { + expect(utils.sprintf('Boolean: %s, Number: %s, Object: %s', true, 42, { key: 'value' })).toBe( + 'Boolean: true, Number: 42, Object: [object Object]' + ); + }); + + it('should handle a mix of strings, functions, and other types', () => { + const dynamicPart = () => 'computed'; + expect(utils.sprintf('String: %s, Function: %s, Number: %s', 'example', dynamicPart, 123)).toBe( + 'String: example, Function: computed, Number: 123' + ); + }); + + it('should handle missing arguments as undefined', () => { + expect(utils.sprintf('Two placeholders: %s and %s', 'first')).toBe('Two placeholders: first and undefined'); + }); + }); +}); diff --git a/src/utils.tsx b/src/utils.tsx index e824740..b1f35bc 100644 --- a/src/utils.tsx +++ b/src/utils.tsx @@ -51,27 +51,6 @@ export function areUsersEqual(user1: UserInfo, user2: UserInfo): boolean { return true; } -export function areObjectsEqual(obj1: any, obj2: any) { - const obj1Keys = Object.keys(obj1); - const obj2Keys = Object.keys(obj2); - - if (obj1Keys.length !== obj2Keys.length) { - return false; - } - - for (let i = 0; i < obj1Keys.length; i += 1) { - const key = obj1Keys[i]; - if (typeof obj1[key] === 'object' && typeof obj2[key] === 'object') { - if (!areObjectsEqual(obj1[key], obj2[key])) { - return false; - } - } else if (obj1[key] !== obj2[key]) { - return false; - } - } - return true; -} - export interface AcceptsForwardedRef { forwardedRef?: React.Ref; } From 3e28e6826d14b4a1dbc6cea7c967033edd2a24aa Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Mon, 9 Sep 2024 20:21:52 +0600 Subject: [PATCH 5/7] [FSSDK-10616] hooks improvement --- src/hooks.spec.tsx | 235 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 221 insertions(+), 14 deletions(-) diff --git a/src/hooks.spec.tsx b/src/hooks.spec.tsx index ff150d1..d43042f 100644 --- a/src/hooks.spec.tsx +++ b/src/hooks.spec.tsx @@ -15,7 +15,6 @@ */ /// - import * as React from 'react'; import { act } from 'react-dom/test-utils'; import { render, renderHook, screen, waitFor } from '@testing-library/react'; @@ -23,8 +22,25 @@ import '@testing-library/jest-dom'; import { OptimizelyProvider } from './Provider'; import { NotReadyReason, OnReadyResult, ReactSDKClient, VariableValuesObject } from './client'; -import { useExperiment, useFeature, useDecision, useTrackEvent, hooksLogger } from './hooks'; +import { useExperiment, useFeature, useDecision, useTrackEvent } from './hooks'; import { OptimizelyDecision } from './utils'; +import { getLogger } from '@optimizely/optimizely-sdk'; + +jest.mock('@optimizely/optimizely-sdk', () => { + const originalModule = jest.requireActual('@optimizely/optimizely-sdk'); + return { + ...originalModule, + getLogger: jest.fn().mockReturnValue({ + error: jest.fn(), + warn: jest.fn(), + info: jest.fn(), + debug: jest.fn(), + }), + }; +}); + +const hooksLogger = getLogger('ReactSDK'); + const defaultDecision: OptimizelyDecision = { enabled: false, variables: {}, @@ -79,7 +95,6 @@ describe('hooks', () => { let forcedVariationUpdateCallbacks: Array<() => void>; let decideMock: jest.Mock; let setForcedDecisionMock: jest.Mock; - let hooksLoggerErrorSpy: jest.SpyInstance; const REJECTION_REASON = 'A rejection reason you should never see in the test runner'; beforeEach(() => { @@ -125,7 +140,6 @@ describe('hooks', () => { forcedVariationUpdateCallbacks = []; decideMock = jest.fn(); setForcedDecisionMock = jest.fn(); - hooksLoggerErrorSpy = jest.spyOn(hooksLogger, 'error'); optimizelyMock = { activate: activateMock, onReady: jest.fn().mockImplementation((config) => getOnReadyPromise(config || {})), @@ -185,7 +199,7 @@ describe('hooks', () => { (res) => res.dataReadyPromise, (err) => null ); - hooksLoggerErrorSpy.mockReset(); + jest.resetAllMocks(); }); describe('useExperiment', () => { @@ -426,6 +440,16 @@ describe('hooks', () => { }); describe('useFeature', () => { + it('should print error if optimizely is not provided', async () => { + render( + // @ts-ignore + + + + ); + await waitFor(() => expect(hooksLogger.error).toHaveBeenCalled()); + }); + it('should render true when the feature is enabled', async () => { isFeatureEnabledMock.mockReturnValue(true); @@ -655,6 +679,160 @@ describe('hooks', () => { }); describe('useDecision', () => { + it('should handle no client promise response', async () => { + getOnReadyPromise = () => + new Promise((resolve) => { + setTimeout(() => { + resolve({ + success: false, + reason: NotReadyReason.NO_CLIENT, + dataReadyPromise: new Promise((r) => setTimeout(() => r({ success: false }), mockDelay)), + }); + }); + }); + decideMock.mockReturnValue({ ...defaultDecision }); + + render( + + + + ); + + await waitFor(() => { + expect(screen.getByTestId('result')).toHaveTextContent('false|{}|false|false'); + expect(hooksLogger.warn).toHaveBeenCalled(); + }); + }); + + it('should handle no client, but data ready promise success', () => { + getOnReadyPromise = () => + new Promise((resolve) => { + setTimeout(() => { + resolve({ + success: false, + reason: NotReadyReason.NO_CLIENT, + dataReadyPromise: new Promise((r) => setTimeout(() => r({ success: true }), mockDelay)), + }); + }); + }); + decideMock.mockReturnValue({ ...defaultDecision }); + + render( + + + + ); + + expect(screen.getByTestId('result')).toHaveTextContent('false|{}|true|false'); + }); + + it('should handle user not ready promise response', async () => { + getOnReadyPromise = () => + new Promise((resolve) => { + setTimeout(() => { + resolve({ + success: false, + reason: NotReadyReason.USER_NOT_READY, + dataReadyPromise: new Promise((r) => setTimeout(() => r({ success: false }), mockDelay)), + }); + }); + }); + decideMock.mockReturnValue({ ...defaultDecision }); + + render( + + + + ); + + await waitFor(() => { + expect(screen.getByTestId('result')).toHaveTextContent('false|{}|false|false'); + expect(hooksLogger.warn).toHaveBeenCalled(); + }); + }); + + it('should handle user not ready, but data ready promise success', () => { + getOnReadyPromise = () => + new Promise((resolve) => { + setTimeout(() => { + resolve({ + success: false, + reason: NotReadyReason.USER_NOT_READY, + dataReadyPromise: new Promise((r) => setTimeout(() => r({ success: true }), mockDelay)), + }); + }); + }); + decideMock.mockReturnValue({ ...defaultDecision }); + + render( + + + + ); + + expect(screen.getByTestId('result')).toHaveTextContent('false|{}|true|false'); + }); + + it('should handle default success false case', async () => { + getOnReadyPromise = () => + new Promise((resolve) => { + setTimeout(() => { + resolve({ + success: false, + reason: 'UNKNOWN', + dataReadyPromise: new Promise((r) => setTimeout(() => r({ success: false }), mockDelay)), + }); + }); + }); + decideMock.mockReturnValue({ ...defaultDecision }); + + render( + + + + ); + + await waitFor(() => { + expect(hooksLogger.warn).toHaveBeenCalled(); + expect(screen.getByTestId('result')).toHaveTextContent('false|{}|false|false'); + }); + }); + + it('should handle default success true case', async () => { + getOnReadyPromise = () => + new Promise((resolve) => { + setTimeout(() => { + resolve({ + success: false, + reason: 'UNKNOWN', + dataReadyPromise: new Promise((r) => setTimeout(() => r({ success: true }), mockDelay)), + }); + }); + }); + decideMock.mockReturnValue({ ...defaultDecision }); + + render( + + + + ); + + await waitFor(() => { + expect(hooksLogger.warn).toHaveBeenCalled(); + expect(screen.getByTestId('result')).toHaveTextContent('false|{}|true|false'); + }); + }); + + it('should print error if optimizely is not provided', async () => { + render( + // @ts-ignore + + + + ); + await waitFor(() => expect(hooksLogger.error).toHaveBeenCalled()); + }); + it('should render true when the flag is enabled', async () => { decideMock.mockReturnValue({ ...defaultDecision, @@ -705,10 +883,34 @@ describe('hooks', () => { variables: { foo: 'bar' }, }); readySuccess = true; + // When timeout is reached, but dataReadyPromise is resolved later with the decision value await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('true|{"foo":"bar"}|true|true')); }); + it('should log warn message if dataReadyPromise resolves as false', async () => { + decideMock.mockReturnValue({ ...defaultDecision }); + readySuccess = false; + mockDelay = 20; + + render( + + + + ); + + // Initial render + expect(screen.getByTestId('result')).toHaveTextContent('false|{}|false|false'); + + readySuccess = false; + + // When timeout is reached, but dataReadyPromise is resolved later with the decision value + await waitFor(() => { + expect(hooksLogger.warn).toHaveBeenCalled(); + expect(screen.getByTestId('result')).toHaveTextContent('false|{}|false|true'); + }); + }); + it('should gracefully handle the client promise rejecting after timeout', async () => { readySuccess = false; decideMock.mockReturnValue({ ...defaultDecision }); @@ -721,7 +923,9 @@ describe('hooks', () => { ); - await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('false|{}|false|false')); + await waitFor(() => { + expect(screen.getByTestId('result')).toHaveTextContent('false|{}|false|false'); + }); }); it('should re-render when the user attributes change using autoUpdate', async () => { @@ -1002,16 +1206,17 @@ describe('hooks', () => { await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('false|{}|true|false')); }); }); + describe('useTrackEvent', () => { it('returns track method and false states when optimizely is not provided', () => { const wrapper = ({ children }: { children: React.ReactNode }): React.ReactElement => { //@ts-ignore return {children}; }; + const { result } = renderHook(() => useTrackEvent(), { wrapper }); - expect(result.current[0]).toBeInstanceOf(Function); - expect(result.current[1]).toBe(false); - expect(result.current[2]).toBe(false); + + expect(result.current).toEqual([expect.any(Function), false, false]); }); it('returns track method along with clientReady and didTimeout state when optimizely instance is provided', () => { @@ -1022,9 +1227,10 @@ describe('hooks', () => { ); const { result } = renderHook(() => useTrackEvent(), { wrapper }); - expect(result.current[0]).toBeInstanceOf(Function); - expect(typeof result.current[1]).toBe('boolean'); - expect(typeof result.current[2]).toBe('boolean'); + result.current[0]('eventKey'); + + expect(optimizelyMock.track).toHaveBeenCalledTimes(1); + expect(result.current).toEqual([expect.any(Function), true, false]); }); it('Log error when track method is called and optimizely instance is not provided', () => { @@ -1034,7 +1240,7 @@ describe('hooks', () => { }; const { result } = renderHook(() => useTrackEvent(), { wrapper }); result.current[0]('eventKey'); - expect(hooksLogger.error).toHaveBeenCalledTimes(1); + expect(hooksLogger.error).toHaveBeenCalled(); }); it('Log error when track method is called and client is not ready', () => { @@ -1048,7 +1254,8 @@ describe('hooks', () => { const { result } = renderHook(() => useTrackEvent(), { wrapper }); result.current[0]('eventKey'); - expect(hooksLogger.error).toHaveBeenCalledTimes(1); + + expect(hooksLogger.error).toHaveBeenCalled(); }); }); }); From 719a834c4774699dfed5350a0b128d9afc873fde Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Mon, 9 Sep 2024 22:47:56 +0600 Subject: [PATCH 6/7] [FSSDK-10616] util test case readability improvement --- src/utils.spec.tsx | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/src/utils.spec.tsx b/src/utils.spec.tsx index 07b95c4..07cad2f 100644 --- a/src/utils.spec.tsx +++ b/src/utils.spec.tsx @@ -25,28 +25,33 @@ describe('utils', () => { it('returns true if users are equal', () => { const user2 = JSON.parse(JSON.stringify(user)); + const areUsersEqual = utils.areUsersEqual(user, user2); - expect(utils.areUsersEqual(user, user2)).toBe(true); + expect(areUsersEqual).toBe(true); }); it('returns false if users are not equal', () => { const user2 = { id: '2', attributes: { name: 'user2' } }; + const areUsersEqual = utils.areUsersEqual(user, user2); - expect(utils.areUsersEqual(user, user2)).toBe(false); + expect(areUsersEqual).toBe(false); }); it('returns false if key lengths are not equal', () => { const user2 = { id: '1', attributes: { name: 'user1', age: 30 } }; + const areUsersEqual = utils.areUsersEqual(user, user2); - expect(utils.areUsersEqual(user, user2)).toBe(false); + expect(areUsersEqual).toBe(false); }); it('returns false if one of the key value pairs are not equal', () => { const user2 = { id: '1', attributes: { name: 'user2' } }; + const areUsersEqual = utils.areUsersEqual(user, user2); - expect(utils.areUsersEqual(user, user2)).toBe(false); + expect(areUsersEqual).toBe(false); }); }); + describe('hoistStaticsAndForwardRefs', () => { class TestComponent extends React.Component<{ forwardedRef?: React.Ref }> { static testStaticMethod = () => 'static method result'; @@ -89,50 +94,57 @@ describe('utils', () => { it('should return true for equal attributes', () => { const attrs1 = { a: 1, b: 2 }; const attrs2 = { a: 1, b: 2 }; + const areAttributesEqual = utils.areAttributesEqual(attrs1, attrs2); - expect(utils.areAttributesEqual(attrs1, attrs2)).toBe(true); + expect(areAttributesEqual).toBe(true); }); it('should return false for different attribute keys', () => { const attrs1 = { a: 1, b: 2 }; const attrs2 = { a: 1, c: 2 }; + const areAttributesEqual = utils.areAttributesEqual(attrs1, attrs2); - expect(utils.areAttributesEqual(attrs1, attrs2)).toBe(false); + expect(areAttributesEqual).toBe(false); }); it('should return false for different attribute values', () => { const attrs1 = { a: 1, b: 2 }; const attrs2 = { a: 1, b: 3 }; + const areAttributesEqual = utils.areAttributesEqual(attrs1, attrs2); - expect(utils.areAttributesEqual(attrs1, attrs2)).toBe(false); + expect(areAttributesEqual).toBe(false); }); it('should return false if the number of attributes differs', () => { const attrs1 = { a: 1, b: 2 }; const attrs2 = { a: 1 }; + const areAttributesEqual = utils.areAttributesEqual(attrs1, attrs2); - expect(utils.areAttributesEqual(attrs1, attrs2)).toBe(false); + expect(areAttributesEqual).toBe(false); }); it('should handle undefined or null attributes as empty objects', () => { const attrs1 = null; const attrs2 = undefined; + const areAttributesEqual = utils.areAttributesEqual(attrs1, attrs2); - expect(utils.areAttributesEqual(attrs1, attrs2)).toBe(true); + expect(areAttributesEqual).toBe(true); }); it('should return false when one attribute is an object and another is not', () => { const attrs1 = { a: 1 }; const attrs2 = 'not an object'; + const areAttributesEqual = utils.areAttributesEqual(attrs1, attrs2); - expect(utils.areAttributesEqual(attrs1, attrs2)).toBe(false); + expect(areAttributesEqual).toBe(false); }); it('should handle different types of attribute values correctly', () => { const attrs1 = { a: '1', b: true }; const attrs2 = { a: '1', b: true }; + const areAttributesEqual = utils.areAttributesEqual(attrs1, attrs2); - expect(utils.areAttributesEqual(attrs1, attrs2)).toBe(true); + expect(areAttributesEqual).toBe(true); }); }); From 1f97dbb32ea2fa578c21458490bac247f6714c00 Mon Sep 17 00:00:00 2001 From: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com> Date: Tue, 10 Sep 2024 18:35:28 +0600 Subject: [PATCH 7/7] [FSSDK-10437] jest coverage report job addition --- .github/workflows/react.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/react.yml b/.github/workflows/react.yml index d03559c..a72f2a7 100644 --- a/.github/workflows/react.yml +++ b/.github/workflows/react.yml @@ -24,6 +24,12 @@ jobs: run: yarn install - name: Run tests run: yarn test + + coverage: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - uses: ArtiomTr/jest-coverage-report-action@v2 integration_tests: name: Run integration tests