Skip to content

Commit

Permalink
Sameeran/ff 1130 sdk assigns subjects to a holdout js common (#27)
Browse files Browse the repository at this point in the history
* Add holdout test and update internal get assignment

* Add holdoutKey and holdoutVariation to the output

* Update test to check that get assignment returns control variation and logs status_quo if subject is in holdout

* Test that get assignment returns the shipped variation and logs all_shipped_variants if subject is in holdout

* Make holdout field nullable

* Make toBeNull consistent

* Restructure holdouts in mock config for test

* Update get assignment code to match new format

* Remove console log

* Change subject shard range to 10000 in tests

* Rename HoldoutVariationType to NullableHoldoutVariationType

* Update makeful to include holdout test data

* Try removing dot

* Fix spelling of directory

* Bump axios version and use nullish coalesce

* Bump version number of sdk
  • Loading branch information
sameerank authored Nov 18, 2023
1 parent d0ab31b commit 4c176e2
Show file tree
Hide file tree
Showing 9 changed files with 352 additions and 35 deletions.
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ test-data:
cp ${gitDataDir}rac-experiments-v3.json ${testDataDir}
cp ${gitDataDir}rac-experiments-v3-obfuscated.json ${testDataDir}
cp -r ${gitDataDir}assignment-v2 ${testDataDir}
cp -r ${gitDataDir}assignment-v2-holdouts/. ${testDataDir}assignment-v2
rm -rf ${tempDir}

## prepare
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@eppo/js-client-sdk-common",
"version": "1.7.0",
"version": "1.8.0",
"description": "Eppo SDK for client-side JavaScript applications (base for both web and react native)",
"main": "dist/index.js",
"files": [
Expand Down Expand Up @@ -63,7 +63,7 @@
"xhr-mock": "^2.5.1"
},
"dependencies": {
"axios": "^0.27.2",
"axios": "^1.6.0",
"lru-cache": "^10.0.1",
"md5": "^2.3.0"
}
Expand Down
17 changes: 17 additions & 0 deletions src/assignment-logger.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
export enum HoldoutVariationEnum {
STATUS_QUO = 'status_quo',
ALL_SHIPPED = 'all_shipped_variants',
}

export type NullableHoldoutVariationType = HoldoutVariationEnum | null;

/**
* Holds data about the variation a subject was assigned to.
* @public
Expand Down Expand Up @@ -33,6 +40,16 @@ export interface IAssignmentEvent {
*/
timestamp: string;

/**
* An Eppo holdout key
*/
holdout: string | null;

/**
* The Eppo holdout variation for the assigned variation
*/
holdoutVariation: NullableHoldoutVariationType;

// eslint-disable-next-line @typescript-eslint/no-explicit-any
subjectAttributes: Record<string, any>;
}
Expand Down
244 changes: 232 additions & 12 deletions src/client/eppo-client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ describe('EppoClient E2E test', () => {
const mockExperimentConfig = {
name: flagKey,
enabled: true,
subjectShards: 100,
subjectShards: 10000,
overrides: {},
typedOverrides: {},
rules: [
Expand All @@ -99,32 +99,35 @@ describe('EppoClient E2E test', () => {
allocations: {
allocation1: {
percentExposure: 1,
statusQuoVariationKey: null,
shippedVariationKey: null,
holdouts: [],
variations: [
{
name: 'control',
value: 'control',
typedValue: 'control',
shardRange: {
start: 0,
end: 34,
end: 3333,
},
},
{
name: 'variant-1',
value: 'variant-1',
typedValue: 'variant-1',
shardRange: {
start: 34,
end: 67,
start: 3333,
end: 6667,
},
},
{
name: 'variant-2',
value: 'variant-2',
typedValue: 'variant-2',
shardRange: {
start: 67,
end: 100,
start: 6667,
end: 10000,
},
},
],
Expand Down Expand Up @@ -470,14 +473,17 @@ describe('EppoClient E2E test', () => {
allocations: {
allocation1: {
percentExposure: 1,
statusQuoVariationKey: null,
shippedVariationKey: null,
holdouts: [],
variations: [
{
name: 'control',
value: 'control',
typedValue: 'control',
shardRange: {
start: 0,
end: 100,
end: 10000,
},
},
{
Expand All @@ -502,6 +508,9 @@ describe('EppoClient E2E test', () => {
allocations: {
allocation1: {
percentExposure: 1,
statusQuoVariationKey: null,
shippedVariationKey: null,
holdouts: [],
variations: [
{
name: 'control',
Expand All @@ -518,7 +527,7 @@ describe('EppoClient E2E test', () => {
typedValue: 'treatment',
shardRange: {
start: 0,
end: 100,
end: 10000,
},
},
],
Expand Down Expand Up @@ -546,14 +555,17 @@ describe('EppoClient E2E test', () => {
allocations: {
allocation1: {
percentExposure: 1,
statusQuoVariationKey: null,
shippedVariationKey: null,
holdouts: [],
variations: [
{
name: 'some-new-treatment',
value: 'some-new-treatment',
typedValue: 'some-new-treatment',
shardRange: {
start: 0,
end: 100,
end: 10000,
},
},
],
Expand Down Expand Up @@ -596,13 +608,221 @@ describe('EppoClient E2E test', () => {

const client = new EppoClient(storage);
let assignment = client.getAssignment('subject-10', flagKey, { appVersion: 9 });
expect(assignment).toEqual(null);
expect(assignment).toBeNull();
assignment = client.getAssignment('subject-10', flagKey);
expect(assignment).toEqual(null);
expect(assignment).toBeNull();
assignment = client.getAssignment('subject-10', flagKey, { appVersion: 11 });
expect(assignment).toEqual('control');
});

it('returns control variation and logs holdout key if subject is in holdout in an experiment allocation', () => {
const entry = {
...mockExperimentConfig,
allocations: {
allocation1: {
percentExposure: 1,
statusQuoVariationKey: 'variation-7',
shippedVariationKey: null,
holdouts: [
{
holdoutKey: 'holdout-2',
statusQuoShardRange: {
start: 0,
end: 200,
},
shippedShardRange: null, // this is an experiment allocation because shippedShardRange is null
},
{
holdoutKey: 'holdout-3',
statusQuoShardRange: {
start: 200,
end: 400,
},
shippedShardRange: null,
},
],
variations: [
{
name: 'control',
value: 'control',
typedValue: 'control',
shardRange: {
start: 0,
end: 3333,
},
variationKey: 'variation-7',
},
{
name: 'variant-1',
value: 'variant-1',
typedValue: 'variant-1',
shardRange: {
start: 3333,
end: 6667,
},
variationKey: 'variation-8',
},
{
name: 'variant-2',
value: 'variant-2',
typedValue: 'variant-2',
shardRange: {
start: 6667,
end: 10000,
},
variationKey: 'variation-9',
},
],
},
},
};

storage.setEntries({ [flagKey]: entry });

const mockLogger = td.object<IAssignmentLogger>();
const client = new EppoClient(storage);
client.setLogger(mockLogger);
td.reset();

// subject-79 --> holdout shard is 186
let assignment = client.getAssignment('subject-79', flagKey);
expect(assignment).toEqual('control');
// Only log holdout key (not variation) if this is an experiment allocation
expect(td.explain(mockLogger.logAssignment).calls[0].args[0].holdoutVariation).toBeNull();
expect(td.explain(mockLogger.logAssignment).calls[0].args[0].holdout).toEqual('holdout-2');

// subject-8 --> holdout shard is 201
assignment = client.getAssignment('subject-8', flagKey);
expect(assignment).toEqual('control');
// Only log holdout key (not variation) if this is an experiment allocation
expect(td.explain(mockLogger.logAssignment).calls[1].args[0].holdoutVariation).toBeNull();
expect(td.explain(mockLogger.logAssignment).calls[1].args[0].holdout).toEqual('holdout-3');

// subject-11 --> holdout shard is 9137 (outside holdout), non-holdout assignment shard is 8414
assignment = client.getAssignment('subject-11', flagKey);
expect(assignment).toEqual('variant-2');
expect(td.explain(mockLogger.logAssignment).calls[2].args[0].holdoutVariation).toBeNull();
expect(td.explain(mockLogger.logAssignment).calls[2].args[0].holdout).toBeNull();
});

it('returns the shipped variation and logs holdout key and variation if subject is in holdout in a rollout allocation', () => {
const entry = {
...mockExperimentConfig,
allocations: {
allocation1: {
percentExposure: 1,
statusQuoVariationKey: 'variation-7',
shippedVariationKey: 'variation-8',
holdouts: [
{
holdoutKey: 'holdout-2',
statusQuoShardRange: {
start: 0,
end: 100,
},
shippedShardRange: {
start: 100,
end: 200,
},
},
{
holdoutKey: 'holdout-3',
statusQuoShardRange: {
start: 200,
end: 300,
},
shippedShardRange: {
start: 300,
end: 400,
},
},
],
variations: [
{
name: 'control',
value: 'control',
typedValue: 'control',
shardRange: {
start: 0,
end: 0,
},
variationKey: 'variation-7',
},
{
name: 'variant-1',
value: 'variant-1',
typedValue: 'variant-1',
shardRange: {
start: 0,
end: 0,
},
variationKey: 'variation-8',
},
{
name: 'variant-2',
value: 'variant-2',
typedValue: 'variant-2',
shardRange: {
start: 0,
end: 10000,
},
variationKey: 'variation-9',
},
],
},
},
};

storage.setEntries({ [flagKey]: entry });

const mockLogger = td.object<IAssignmentLogger>();
const client = new EppoClient(storage);
client.setLogger(mockLogger);
td.reset();

// subject-227 --> holdout shard is 57
let assignment = client.getAssignment('subject-227', flagKey);
expect(assignment).toEqual('control');
// Log both holdout key and variation if this is a rollout allocation
expect(td.explain(mockLogger.logAssignment).calls[0].args[0].holdoutVariation).toEqual(
'status_quo',
);
expect(td.explain(mockLogger.logAssignment).calls[0].args[0].holdout).toEqual('holdout-2');

// subject-79 --> holdout shard is 186
assignment = client.getAssignment('subject-79', flagKey);
expect(assignment).toEqual('variant-1');
// Log both holdout key and variation if this is a rollout allocation
expect(td.explain(mockLogger.logAssignment).calls[1].args[0].holdoutVariation).toEqual(
'all_shipped_variants',
);
expect(td.explain(mockLogger.logAssignment).calls[1].args[0].holdout).toEqual('holdout-2');

// subject-8 --> holdout shard is 201
assignment = client.getAssignment('subject-8', flagKey);
expect(assignment).toEqual('control');
// Log both holdout key and variation if this is a rollout allocation
expect(td.explain(mockLogger.logAssignment).calls[2].args[0].holdoutVariation).toEqual(
'status_quo',
);
expect(td.explain(mockLogger.logAssignment).calls[2].args[0].holdout).toEqual('holdout-3');

// subject-50 --> holdout shard is 347
assignment = client.getAssignment('subject-50', flagKey);
expect(assignment).toEqual('variant-1');
// Log both holdout key and variation if this is a rollout allocation
expect(td.explain(mockLogger.logAssignment).calls[3].args[0].holdoutVariation).toEqual(
'all_shipped_variants',
);
expect(td.explain(mockLogger.logAssignment).calls[3].args[0].holdout).toEqual('holdout-3');

// subject-7 --> holdout shard is 9483 (outside holdout), non-holdout assignment shard is 8673
assignment = client.getAssignment('subject-7', flagKey);
expect(assignment).toEqual('variant-2');
expect(td.explain(mockLogger.logAssignment).calls[4].args[0].holdoutVariation).toBeNull();
expect(td.explain(mockLogger.logAssignment).calls[4].args[0].holdout).toBeNull();
});

function getAssignmentsWithSubjectAttributes(
subjectsWithAttributes: {
subjectKey: string;
Expand Down Expand Up @@ -732,7 +952,7 @@ describe('EppoClient E2E test', () => {
},
);

expect(variation).not.toEqual(null);
expect(variation).not.toBeNull();
expect(td.explain(mockLogger.logAssignment).callCount).toEqual(1);
});
});
Expand Down
Loading

0 comments on commit 4c176e2

Please sign in to comment.