Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[SDKS-7563] Stabilize integration tests #762

Merged
merged 3 commits into from
Oct 2, 2023
Merged

[SDKS-7563] Stabilize integration tests #762

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
33cdcab
[SDKS-7563] stabilize integration tests for flag sets
Sep 28, 2023
a2fa07a
Update commons rc version
Sep 28, 2023
b7b8434
Improve ready from cache tests
Oct 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@splitsoftware/splitio",
"version": "10.23.1",
"version": "10.23.2-rc.0",
"description": "Split SDK",
"files": [
"README.md",
Expand Down Expand Up @@ -40,7 +40,7 @@
"node": ">=6"
},
"dependencies": {
"@splitsoftware/splitio-commons": "1.9.1",
"@splitsoftware/splitio-commons": "1.9.2-rc.0",
"@types/google.analytics": "0.0.40",
"@types/ioredis": "^4.28.0",
"bloom-filters": "^3.0.0",
Expand Down
3 changes: 2 additions & 1 deletion src/__tests__/browserSuites/manager.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ export default async function (settings, fetchMock, assert) {
'killed': mockSplits.splits[index].killed,
'changeNumber': mockSplits.splits[index].changeNumber,
'treatments': map(mockSplits.splits[index].conditions[0].partitions, partition => partition.treatment),
'configs': mockSplits.splits[index].configurations || {}
'configs': mockSplits.splits[index].configurations || {},
'sets': mockSplits.splits[index].sets || []
});

assert.equal(manager.split('non_existent'), null, 'Trying to get a manager.split() of a Split that does not exist returns null.');
Expand Down
51 changes: 44 additions & 7 deletions src/__tests__/browserSuites/ready-from-cache.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ export default function (fetchMock, assert) {
events: 'https://events.baseurl/readyFromCache_5'
};
localStorage.clear();
t.plan(7);
t.plan(6);

fetchMock.getOnce(testUrls.sdk + '/splitChanges?since=-1&names=p1__split,p2__split', { status: 200, body: { splits: [splitDeclarations.p1__split, splitDeclarations.p2__split], since: -1, till: 1457552620999 } }, { delay: 10 }); // short delay to let emit SDK_READY_FROM_CACHE
// fetchMock.getOnce(testUrls.sdk + '/splitChanges?since=1457552620999&names=p1__split', { status: 200, body: { splits: [], since: 1457552620999, till: 1457552620999 } });
Expand All @@ -504,7 +504,8 @@ export default function (fetchMock, assert) {
const manager = splitio.manager();

client.once(client.Event.SDK_READY_FROM_CACHE, () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than simply removing asserts, it is better to update them to keep information of the expected behaviour:

client.once(client.Event.SDK_READY_FROM_CACHE, () => {
  t.fail('It should not emit SDK_READY_FROM_CACHE because ...');
  t.end();
});

Idem for the other removed asserts.

Copy link
Contributor

@EmilianoSanchez EmilianoSanchez Sep 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this change, none of the tests below line 472 (/** Fetch specific splits **/) is asserting that the SDK_READY_FROM_CACHE event is emitted when using split filters.

So, I would create a new test for the following basic case:

  assert.test(t => { // Testing when we start with cached data with split filter, and the same split filter is provided in the config

This should emit the event because the config has the same split filter as the one in the cache.

t.deepEqual(manager.names(), ['p2__split'], 'stored p3__split must be removed because doesn\'t match the filter');
t.fail('It should not emit SDK_READY_FROM_CACHE because localstorage is cleaned and there isn\'t cached data');
t.end();
});

client.once(client.Event.SDK_READY, () => {
Expand All @@ -521,6 +522,46 @@ export default function (fetchMock, assert) {
});
});

assert.test(t => { // Testing when we start with cached data with split filter, and the same split filter is provided in the config
const testUrls = {
sdk: 'https://sdk.baseurl/readyFromCache_5',
events: 'https://events.baseurl/readyFromCache_5'
};

localStorage.clear();
t.plan(1);

fetchMock.getOnce(testUrls.sdk + '/splitChanges?since=25&names=p2__split,p3__split', { status: 200, body: { splits: [splitDeclarations.p2__split, splitDeclarations.p3__split], since: -1, till: 1457552620999 } }, { delay: 10 }); // short delay to let emit SDK_READY_FROM_CACHE
fetchMock.getOnce(testUrls.sdk + '/mySegments/nicolas%40split.io', { status: 200, body: { mySegments: [] } });

localStorage.setItem('some_user_item', 'user_item');
localStorage.setItem('readyFromCache_5.SPLITIO.splits.till', 25);
localStorage.setItem('readyFromCache_5.SPLITIO.splits.filterQuery', '&names=p2__split,p3__split');
localStorage.setItem('readyFromCache_5.SPLITIO.split.p2__split', JSON.stringify(splitDeclarations.p2__split));
localStorage.setItem('readyFromCache_5.SPLITIO.split.p3__split', JSON.stringify(splitDeclarations.p3__split));

const splitio = SplitFactory({
...baseConfig,
storage: {
type: 'LOCALSTORAGE',
prefix: 'readyFromCache_5'
},
urls: testUrls,
sync: {
splitFilters: [{ type: 'byName', values: ['p2__split', 'p3__split'] }]
},
debug: true
});
const client = splitio.client();

client.once(client.Event.SDK_READY_FROM_CACHE, () => {
t.assert('It should emit SDK_READY_FROM_CACHE because there is cached data for the same queryFilter');
client.destroy().then(() => {
t.end();
});
});
});

assert.test(t => { // Testing when we start from scratch, and a valid split filter config
const testUrls = {
sdk: 'https://sdk.baseurl/readyFromCache_5B',
Expand Down Expand Up @@ -740,7 +781,7 @@ export default function (fetchMock, assert) {
events: 'https://events.baseurl/readyFromCache_9'
};
localStorage.clear();
t.plan(7);
t.plan(6);

fetchMock.getOnce(testUrls.sdk + '/splitChanges?since=-1&names=no%20exist%20trim,no_exist,p3__split&prefixes=no%20exist%20trim,p2', { status: 200, body: { splits: [splitDeclarations.p2__split, splitDeclarations.p3__split], since: -1, till: 1457552620999 } }, { delay: 10 }); // short delay to let emit SDK_READY_FROM_CACHE
fetchMock.getOnce(testUrls.sdk + '/mySegments/nicolas%40split.io', { status: 200, body: { mySegments: [] } });
Expand All @@ -766,10 +807,6 @@ export default function (fetchMock, assert) {
const client = splitio.client();
const manager = splitio.manager();

client.once(client.Event.SDK_READY_FROM_CACHE, () => {
t.deepEqual(manager.names(), ['p2__split'], 'stored p1__split must be removed because doesn\'t match the filter');
});

client.once(client.Event.SDK_READY, () => {
t.deepEqual(manager.names(), ['p3__split', 'p2__split'], 'active splits should be present for evaluation');

Expand Down
5 changes: 3 additions & 2 deletions src/__tests__/browserSuites/telemetry.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export default async function telemetryBrowserSuite(fetchMock, assert) {
const data = JSON.parse(opts.body);

// Validate last successful sync
assert.deepEqual(Object.keys(data.lS), ['sp', 'ms', 'te'], 'Successful splitChanges, mySegments and metrics/config requests');
assert.deepEqual(Object.keys(data.lS), ['ms', 'sp', 'te'], 'Successful splitChanges, mySegments and metrics/config requests');
lastSync = data.lS; delete data.lS;

// Validate http and method latencies
Expand Down Expand Up @@ -106,7 +106,8 @@ export default async function telemetryBrowserSuite(fetchMock, assert) {
oM: 0, st: 'memory', aF: 1, rF: 0, sE: false,
rR: { sp: 99999, ms: 60, im: 300, ev: 60, te: 1 } /* override featuresRefreshRate */,
uO: { s: true, e: true, a: false, st: false, t: true } /* override sdk, events and telemetry URLs */,
iQ: 30000, eQ: 500, iM: 0, iL: false, hP: false, nR: 1 /* 1 non ready usage */, t: [], i: [], uC: 2 /* Default GRANTED */
iQ: 30000, eQ: 500, iM: 0, iL: false, hP: false, nR: 1 /* 1 non ready usage */, t: [], i: [], uC: 2 /* Default GRANTED */,
fsT: 0, fsI: 0
}, 'metrics/config JSON payload should be the expected');

finish.next();
Expand Down
3 changes: 2 additions & 1 deletion src/__tests__/nodeSuites/manager.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ export default async function (settings, fetchMock, assert) {
'killed': mockSplits.splits[index].killed,
'changeNumber': mockSplits.splits[index].changeNumber,
'treatments': map(mockSplits.splits[index].conditions[0].partitions, partition => partition.treatment),
'configs': mockSplits.splits[index].configurations || {}
'configs': mockSplits.splits[index].configurations || {},
'sets': mockSplits.splits[index].sets || []
});

assert.equal(manager.split('non_existent'), null, 'Trying to get a manager.split() of a Split that does not exist returns null.');
Expand Down
3 changes: 2 additions & 1 deletion src/__tests__/nodeSuites/telemetry.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ export default async function telemetryNodejsSuite(key, fetchMock, assert) {
oM: 0, st: 'memory', aF: 1, rF: 0, sE: false,
rR: { sp: 99999, se: 60, im: 300, ev: 60, te: 1 } /* override featuresRefreshRate */,
uO: { s: true, e: true, a: false, st: false, t: true } /* override sdk, events and telemetry URLs */,
iQ: 30000, eQ: 500, iM: 0, iL: false, hP: false, nR: 1 /* 1 non ready usage */, t: [], uC: 0 /* NA */
iQ: 30000, eQ: 500, iM: 0, iL: false, hP: false, nR: 1 /* 1 non ready usage */, t: [], uC: 0 /* NA */,
fsT: 0, fsI: 0
}, 'metrics/config JSON payload should be the expected');

finish.next();
Expand Down
9 changes: 5 additions & 4 deletions src/__tests__/offline/browser.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ tape('Browser offline mode', function (assert) {
testing_split: 'on',
testing_split_with_config: {
treatment: 'off',
config: '{ "color": "blue" }'
config: '{ "color": "blue" }',
sets: []
}
};

Expand Down Expand Up @@ -166,10 +167,10 @@ tape('Browser offline mode', function (assert) {

// Manager tests
const expectedSplitView1 = {
name: 'testing_split', trafficType: 'localhost', killed: false, changeNumber: 0, treatments: ['on'], configs: {}
name: 'testing_split', trafficType: 'localhost', killed: false, changeNumber: 0, treatments: ['on'], configs: {}, sets: []
};
const expectedSplitView2 = {
name: 'testing_split_with_config', trafficType: 'localhost', killed: false, changeNumber: 0, treatments: ['off'], configs: { off: '{ "color": "blue" }' }
name: 'testing_split_with_config', trafficType: 'localhost', killed: false, changeNumber: 0, treatments: ['off'], configs: { off: '{ "color": "blue" }' }, sets: []
};
assert.deepEqual(manager.names(), ['testing_split', 'testing_split_with_config']);
assert.deepEqual(manager.split('testing_split'), expectedSplitView1);
Expand Down Expand Up @@ -267,7 +268,7 @@ tape('Browser offline mode', function (assert) {

// Manager tests
const expectedSplitView3 = {
name: 'testing_split_with_config', trafficType: 'localhost', killed: false, changeNumber: 0, treatments: ['nope'], configs: {}
name: 'testing_split_with_config', trafficType: 'localhost', killed: false, changeNumber: 0, treatments: ['nope'], configs: {}, sets: []
};
assert.deepEqual(manager.names(), ['testing_split', 'testing_split_2', 'testing_split_3', 'testing_split_with_config']);
assert.deepEqual(manager.split('testing_split'), expectedSplitView1);
Expand Down
27 changes: 17 additions & 10 deletions src/__tests__/offline/node.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,15 +249,18 @@ function ManagerDotSplitTests(assert) {

const expectedView1 = {
name: 'testing_split', changeNumber: 0, killed: false, trafficType: 'localhost',
treatments: ['on'], configs: {}
treatments: ['on'], configs: {},
sets: []
};
const expectedView2 = {
name: 'testing_split2', changeNumber: 0, killed: false, trafficType: 'localhost',
treatments: ['off'], configs: {}
treatments: ['off'], configs: {},
sets: []
};
const expectedView3 = {
name: 'testing_split3', changeNumber: 0, killed: false, trafficType: 'localhost',
treatments: ['custom_treatment'], configs: {}
treatments: ['custom_treatment'], configs: {},
sets: []
};

assert.deepEqual(manager.split('testing_split'), expectedView1);
Expand Down Expand Up @@ -289,15 +292,17 @@ function ManagerDotYamlTests(mockFileName, assert) {
killed: false,
trafficType: 'localhost',
treatments: ['on'],
configs: {}
configs: {},
sets: []
};
const expectedView2 = {
name: 'testing_split_only_wl',
changeNumber: 0,
killed: false,
trafficType: 'localhost',
treatments: ['whitelisted'],
configs: {}
configs: {},
sets: []
};
const expectedView3 = {
name: 'testing_split_with_wl',
Expand All @@ -308,13 +313,15 @@ function ManagerDotYamlTests(mockFileName, assert) {
configs: {
not_in_whitelist: '{"color": "green"}',
multi_key_wl: '{"color": "brown"}'
}
},
sets: []
};
const expectedView4 = {
name: 'testing_split_off_with_config', changeNumber: 0, killed: false, trafficType: 'localhost',
treatments: ['off'], configs: {
off: '{"color": "green"}'
}
},
sets: []
};

assert.deepEqual(manager.split('testing_split_on'), expectedView1);
Expand Down Expand Up @@ -395,15 +402,15 @@ function MultipleInstancesTests(assert) {

const expectedView1 = {
name: 'testing_split', changeNumber: 0, killed: false, trafficType: 'localhost',
treatments: ['on'], configs: {}
treatments: ['on'], configs: {}, sets: []
};
const expectedView2 = {
name: 'testing_split2', changeNumber: 0, killed: false, trafficType: 'localhost',
treatments: ['off'], configs: {}
treatments: ['off'], configs: {}, sets: []
};
const expectedView3 = {
name: 'testing_split3', changeNumber: 0, killed: false, trafficType: 'localhost',
treatments: ['custom_treatment'], configs: {}
treatments: ['custom_treatment'], configs: {}, sets: []
};

assert.deepEqual(manager.split('testing_split'), expectedView1);
Expand Down
2 changes: 1 addition & 1 deletion src/settings/defaults/version.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export const packageVersion = '10.23.1';
export const packageVersion = '10.23.2-rc.0';
Loading