-
Notifications
You must be signed in to change notification settings - Fork 83
[FSSDK-11529] Impression event logic adjustment for holdouts #1076
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 14 commits
6412e89
3a90a81
f6c4cda
9a77240
b8eac89
4c88648
aa3381e
0d034b2
78eaa41
de3baaa
8e33597
0618802
83b7017
fc8181f
894af95
4fc5339
eb3df08
504bb8c
6bf661a
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 | ||||
---|---|---|---|---|---|---|
|
@@ -29,6 +29,7 @@ import { DECISION_SOURCES } from '../utils/enums'; | |||||
import OptimizelyUserContext from '../optimizely_user_context'; | ||||||
import { newErrorDecision } from '../optimizely_decision'; | ||||||
import { ImpressionEvent } from '../event_processor/event_builder/user_event'; | ||||||
import { OptimizelyDecideOption } from '../shared_types'; | ||||||
|
||||||
describe('Optimizely', () => { | ||||||
const eventDispatcher = { | ||||||
junaed-optimizely marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
@@ -212,5 +213,252 @@ describe('Optimizely', () => { | |||||
const event = processSpy.mock.calls[0][0] as ImpressionEvent; | ||||||
expect(event.cmabUuid).toBe('uuid-cmab'); | ||||||
}); | ||||||
|
||||||
it('should dispatch impression event for holdout decision', async () => { | ||||||
const datafile = getDecisionTestDatafile(); | ||||||
|
||||||
datafile.holdouts = [ | ||||||
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. The holdout data structure is duplicated across multiple test cases. Consider extracting this into a shared test helper or factory function to reduce code duplication. Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||
{ | ||||||
id: 'holdout_test_id', | ||||||
key: 'holdout_test_key', | ||||||
status: 'Running', | ||||||
includeFlags: [], | ||||||
excludeFlags: [], | ||||||
audienceIds: [], | ||||||
audienceConditions: [], | ||||||
variations: [ | ||||||
{ | ||||||
id: 'holdout_variation_id', | ||||||
key: 'holdout_variation_key', | ||||||
variables: [], | ||||||
featureEnabled: false | ||||||
} | ||||||
], | ||||||
trafficAllocation: [ | ||||||
{ | ||||||
entityId: 'holdout_variation_id', | ||||||
endOfRange: 10000 | ||||||
} | ||||||
] | ||||||
} | ||||||
]; | ||||||
|
||||||
const projectConfig = createProjectConfig(datafile); | ||||||
|
||||||
const projectConfigManager = getMockProjectConfigManager({ | ||||||
initConfig: projectConfig, | ||||||
}); | ||||||
|
||||||
const mockEventDispatcher = { | ||||||
dispatchEvent: vi.fn(() => Promise.resolve({ statusCode: 200 })), | ||||||
}; | ||||||
const eventProcessor = getForwardingEventProcessor(mockEventDispatcher); | ||||||
const processSpy = vi.spyOn(eventProcessor, 'process'); | ||||||
|
||||||
const optimizely = new Optimizely({ | ||||||
clientEngine: 'node-sdk', | ||||||
projectConfigManager, | ||||||
eventProcessor, | ||||||
jsonSchemaValidator, | ||||||
logger, | ||||||
odpManager, | ||||||
disposable: true, | ||||||
cmabService: {} as any | ||||||
}); | ||||||
|
||||||
junaed-optimizely marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
// eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||||||
// @ts-ignore | ||||||
const decisionService = optimizely.decisionService; | ||||||
vi.spyOn(decisionService, 'resolveVariationsForFeatureList').mockImplementation(() => { | ||||||
return Value.of('async', [{ | ||||||
error: false, | ||||||
result: { | ||||||
variation: projectConfig.holdouts[0].variations[0], | ||||||
experiment: projectConfig.holdouts[0], | ||||||
decisionSource: DECISION_SOURCES.HOLDOUT, | ||||||
}, | ||||||
reasons: [], | ||||||
}]); | ||||||
}); | ||||||
|
||||||
const user = new OptimizelyUserContext({ | ||||||
optimizely, | ||||||
userId: 'test_user', | ||||||
attributes: {}, | ||||||
}); | ||||||
|
||||||
const decision = await optimizely.decideAsync(user, 'flag_1', []); | ||||||
|
||||||
expect(decision.ruleKey).toBe('holdout_test_key'); | ||||||
expect(decision.flagKey).toBe('flag_1'); | ||||||
expect(decision.variationKey).toBe('holdout_variation_key'); | ||||||
expect(decision.enabled).toBe(false); | ||||||
|
||||||
expect(eventProcessor.process).toHaveBeenCalledOnce(); | ||||||
|
||||||
const event = processSpy.mock.calls[0][0] as ImpressionEvent; | ||||||
|
||||||
expect(event.type).toBe('impression'); | ||||||
expect(event.ruleKey).toBe('holdout_test_key'); | ||||||
expect(event.ruleType).toBe('holdout'); | ||||||
expect(event.enabled).toBe(false); | ||||||
}); | ||||||
|
||||||
it('should not dispatch impression event for holdout when DISABLE_DECISION_EVENT is used', async () => { | ||||||
const datafile = getDecisionTestDatafile(); | ||||||
|
||||||
datafile.holdouts = [ | ||||||
{ | ||||||
id: 'holdout_test_id', | ||||||
key: 'holdout_test_key', | ||||||
status: 'Running', | ||||||
includeFlags: [], | ||||||
excludeFlags: [], | ||||||
audienceIds: [], | ||||||
audienceConditions: [], | ||||||
variations: [ | ||||||
{ | ||||||
id: 'holdout_variation_id', | ||||||
key: 'holdout_variation_key', | ||||||
variables: [], | ||||||
featureEnabled: false | ||||||
} | ||||||
], | ||||||
trafficAllocation: [ | ||||||
{ | ||||||
entityId: 'holdout_variation_id', | ||||||
endOfRange: 10000 | ||||||
} | ||||||
] | ||||||
} | ||||||
]; | ||||||
|
||||||
const projectConfig = createProjectConfig(datafile); | ||||||
|
||||||
const projectConfigManager = getMockProjectConfigManager({ | ||||||
initConfig: projectConfig, | ||||||
}); | ||||||
|
||||||
const mockEventDispatcher = { | ||||||
dispatchEvent: vi.fn(() => Promise.resolve({ statusCode: 200 })), | ||||||
}; | ||||||
const eventProcessor = getForwardingEventProcessor(mockEventDispatcher); | ||||||
vi.spyOn(eventProcessor, 'process'); | ||||||
|
||||||
const optimizely = new Optimizely({ | ||||||
clientEngine: 'node-sdk', | ||||||
projectConfigManager, | ||||||
eventProcessor, | ||||||
jsonSchemaValidator, | ||||||
logger, | ||||||
odpManager, | ||||||
disposable: true, | ||||||
cmabService: {} as any | ||||||
}); | ||||||
|
||||||
// eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||||||
// @ts-ignore | ||||||
const decisionService = optimizely.decisionService; | ||||||
vi.spyOn(decisionService, 'resolveVariationsForFeatureList').mockImplementation(() => { | ||||||
return Value.of('async', [{ | ||||||
error: false, | ||||||
result: { | ||||||
variation: projectConfig.holdouts![0].variations[0], | ||||||
experiment: projectConfig.holdouts![0], | ||||||
decisionSource: DECISION_SOURCES.HOLDOUT, | ||||||
}, | ||||||
reasons: [], | ||||||
}]); | ||||||
}); | ||||||
|
||||||
const user = new OptimizelyUserContext({ | ||||||
optimizely, | ||||||
userId: 'test_user', | ||||||
attributes: {}, | ||||||
}); | ||||||
|
||||||
const decision = await optimizely.decideAsync(user, 'flag_1', [OptimizelyDecideOption.DISABLE_DECISION_EVENT]); | ||||||
|
||||||
expect(decision.ruleKey).toBe('holdout_test_key'); | ||||||
expect(decision.enabled).toBe(false); | ||||||
expect(eventProcessor.process).not.toHaveBeenCalled(); | ||||||
}); | ||||||
}); | ||||||
describe('isFeatureEnabled', () => { | ||||||
it('should dispatch impression event for holdout decision', async () => { | ||||||
junaed-optimizely marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
const datafile = getDecisionTestDatafile(); | ||||||
datafile.holdouts = [ | ||||||
{ | ||||||
id: 'holdout_test_id', | ||||||
key: 'holdout_test_key', | ||||||
status: 'Running', | ||||||
includeFlags: [], | ||||||
excludeFlags: [], | ||||||
audienceIds: [], | ||||||
audienceConditions: [], | ||||||
variations: [ | ||||||
{ | ||||||
id: 'holdout_variation_id', | ||||||
key: 'holdout_variation_key', | ||||||
variables: [], | ||||||
featureEnabled: false | ||||||
} | ||||||
], | ||||||
trafficAllocation: [ | ||||||
{ | ||||||
entityId: 'holdout_variation_id', | ||||||
endOfRange: 10000 | ||||||
} | ||||||
] | ||||||
} | ||||||
]; | ||||||
|
||||||
const projectConfig = createProjectConfig(datafile); | ||||||
const projectConfigManager = getMockProjectConfigManager({ | ||||||
initConfig: projectConfig, | ||||||
}); | ||||||
const mockEventDispatcher = { | ||||||
dispatchEvent: vi.fn(() => Promise.resolve({ statusCode: 200 })), | ||||||
}; | ||||||
const eventProcessor = getForwardingEventProcessor(mockEventDispatcher); | ||||||
vi.spyOn(eventProcessor, 'process'); | ||||||
|
||||||
const optimizely = new Optimizely({ | ||||||
clientEngine: 'node-sdk', | ||||||
projectConfigManager, | ||||||
eventProcessor, | ||||||
jsonSchemaValidator, | ||||||
logger, | ||||||
odpManager, | ||||||
disposable: true, | ||||||
cmabService: {} as any | ||||||
}); | ||||||
|
||||||
// eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||||||
// @ts-ignore | ||||||
const decisionService = optimizely.decisionService; | ||||||
vi.spyOn(decisionService, 'getVariationForFeature').mockReturnValue({ | ||||||
error: false, | ||||||
result: { | ||||||
variation: projectConfig.holdouts![0].variations[0], | ||||||
experiment: projectConfig.holdouts![0], | ||||||
decisionSource: DECISION_SOURCES.HOLDOUT, | ||||||
}, | ||||||
reasons: [], | ||||||
}); | ||||||
const result = optimizely.isFeatureEnabled('flag_1', 'test_user', {}); | ||||||
|
||||||
expect(result).toBe(false); | ||||||
|
||||||
expect(eventProcessor.process).toHaveBeenCalledOnce(); | ||||||
// eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||||||
// @ts-ignore | ||||||
const event = eventProcessor.process.mock.calls[0][0] as ImpressionEvent; | ||||||
|
||||||
expect(event.type).toBe('impression'); | ||||||
expect(event.ruleKey).toBe('holdout_test_key'); | ||||||
expect(event.ruleType).toBe('holdout'); | ||||||
expect(event.enabled).toBe(false); | ||||||
}); | ||||||
}) | ||||||
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. Missing semicolon after the closing brace. This is inconsistent with the rest of the codebase style.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||
}); |
Uh oh!
There was an error while loading. Please reload this page.