Skip to content

Commit

Permalink
Merge branch 'impressions_toggle_baseline' into impressions_toggle_ma…
Browse files Browse the repository at this point in the history
…nager
  • Loading branch information
EmilianoSanchez committed Dec 12, 2024
2 parents 84238a2 + 0d3b71b commit 045e38b
Show file tree
Hide file tree
Showing 9 changed files with 18 additions and 24 deletions.
4 changes: 2 additions & 2 deletions src/listeners/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export class NodeSignalListener implements ISignalListener {
// Cleaned up, remove handlers.
this.stop();

// This handler prevented the default behaviour, start again.
// This handler prevented the default behavior, start again.
// eslint-disable-next-line no-undef
process.kill(process.pid, SIGTERM);
};
Expand All @@ -72,7 +72,7 @@ export class NodeSignalListener implements ISignalListener {
}

if (thenable(handlerResult)) {
// Always exit, even with errors. The promise is returned for UT purposses.
// Always exit, even with errors. The promise is returned for UT purposes.
return handlerResult.then(wrapUp).catch(wrapUp);
} else {
wrapUp();
Expand Down
8 changes: 4 additions & 4 deletions src/logger/__tests__/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,22 +97,22 @@ function testLogLevels(levelToTest: SplitIO.LogLevel) {

}

test('SPLIT LOGGER / Logger class public methods behaviour - instance.debug', () => {
test('SPLIT LOGGER / Logger class public methods behavior - instance.debug', () => {
testLogLevels(LogLevels.DEBUG);

});

test('SPLIT LOGGER / Logger class public methods behaviour - instance.info', () => {
test('SPLIT LOGGER / Logger class public methods behavior - instance.info', () => {
testLogLevels(LogLevels.INFO);

});

test('SPLIT LOGGER / Logger class public methods behaviour - instance.warn', () => {
test('SPLIT LOGGER / Logger class public methods behavior - instance.warn', () => {
testLogLevels(LogLevels.WARN);

});

test('SPLIT LOGGER / Logger class public methods behaviour - instance.error', () => {
test('SPLIT LOGGER / Logger class public methods behavior - instance.error', () => {
testLogLevels(LogLevels.ERROR);

});
Expand Down
2 changes: 2 additions & 0 deletions src/readiness/__tests__/readinessManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,8 @@ test('READINESS MANAGER / Cancel timeout if ready fired', (done) => {

const readinessManager = readinessManagerFactory(EventEmitter, settingsWithTimeout);
readinessManager.init(); // Start the timeout
readinessManager.destroy(); // Should cancel the timeout
readinessManager.init(); // Start the timeout again

readinessManager.gate.on(SDK_READY_TIMED_OUT, () => { sdkReadyTimedoutCalled = true; });
readinessManager.gate.once(SDK_READY, () => { sdkReadyCalled = true; });
Expand Down
2 changes: 1 addition & 1 deletion src/readiness/__tests__/sdkReadinessManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ describe('SDK Readiness Manager - Event emitter', () => {

describe('SDK Readiness Manager - Ready promise', () => {

test('.ready() promise behaviour for clients', async () => {
test('.ready() promise behavior for clients', async () => {
const sdkReadinessManager = sdkReadinessManagerFactory(EventEmitterMock, fullSettings);

const ready = sdkReadinessManager.sdkStatus.ready();
Expand Down
2 changes: 1 addition & 1 deletion src/storages/inRedis/__tests__/RedisAdapter.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ describe('STORAGE Redis Adapter', () => {

expect(ioredisMock.once).toBeCalledTimes(2); // If the method was called, it should have called the `once` function twice. If that it the case we can assume that the method was called on creation.

// Reset stubs again, we'll check the behaviour calling the method directly.
// Reset stubs again, we'll check the behavior calling the method directly.
clearAllMocks();
expect(ioredisMock.once).not.toBeCalled(); // Control assertion
expect(ioredisMock[METHODS_TO_PROMISE_WRAP[METHODS_TO_PROMISE_WRAP.length - 1]]).not.toBeCalled(); // Control assertion
Expand Down
18 changes: 5 additions & 13 deletions src/trackers/__tests__/impressionsTracker.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ const fakeSettingsWithListener = {
};
const fakeWhenInit = (cb: () => void) => cb();

// @TODO validate logic with `impression.track` false
const fakeNoneStrategy = {
process: jest.fn(() => false)
};
Expand All @@ -53,13 +52,6 @@ describe('Impressions Tracker', () => {

const strategy = strategyDebugFactory(impressionObserverCSFactory());

test('Tracker API', () => {
expect(typeof impressionsTrackerFactory).toBe('function'); // The module should return a function which acts as a factory.

const instance = impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy,strategy, fakeWhenInit);
expect(typeof instance.track).toBe('function'); // The instance should implement the track method which will actually track queued impressions.
});

test('Should be able to track impressions (in DEBUG mode without Previous Time).', () => {
const tracker = impressionsTrackerFactory(fakeSettings, fakeImpressionsCache, fakeNoneStrategy, strategy, fakeWhenInit);

Expand All @@ -75,9 +67,9 @@ describe('Impressions Tracker', () => {

expect(fakeImpressionsCache.track).not.toBeCalled(); // cache method should not be called by just creating a tracker

tracker.track([{ imp: imp1 }, { imp: imp2 }, { imp: imp3 }]);
tracker.track([{ imp: imp1 }, { imp: imp2, track: true }, { imp: imp3, track: false }]);

expect(fakeImpressionsCache.track.mock.calls[0][0]).toEqual([imp1, imp2, imp3]); // Should call the storage track method once we invoke .track() method, passing queued params in a sequence.
expect(fakeImpressionsCache.track.mock.calls[0][0]).toEqual([imp1, imp2]); // Should call the storage track method once we invoke .track() method, passing impressions with `track` enabled
});

test('Tracked impressions should be sent to impression listener and integration manager when we invoke .track()', (done) => {
Expand All @@ -98,9 +90,9 @@ describe('Impressions Tracker', () => {
expect(fakeIntegrationsManager.handleImpression).not.toBeCalled(); // The integrations manager handleImpression method should not be invoked if we haven't tracked impressions.

// We signal that we actually want to track the queued impressions.
tracker.track([{ imp: fakeImpression }, { imp: fakeImpression2 }], fakeAttributes);
tracker.track([{ imp: fakeImpression }, { imp: fakeImpression2, track: false }], fakeAttributes);

expect(fakeImpressionsCache.track.mock.calls[0][0]).toEqual([fakeImpression, fakeImpression2]); // Even with a listener, impression should be sent to the cache
expect(fakeImpressionsCache.track.mock.calls[0][0]).toEqual([fakeImpression]); // Even with a listener, impressions (with `track` enabled) should be sent to the cache
expect(fakeListener.logImpression).not.toBeCalled(); // The listener should not be executed synchronously.
expect(fakeIntegrationsManager.handleImpression).not.toBeCalled(); // The integrations manager handleImpression method should not be executed synchronously.

Expand Down Expand Up @@ -162,7 +154,7 @@ describe('Impressions Tracker', () => {
expect(fakeImpressionsCache.track).not.toBeCalled(); // storage method should not be called until impressions are tracked.

trackers.forEach(tracker => {
tracker.track([{ imp: impression }, { imp: impression2 }, { imp: impression3 }]);
tracker.track([{ imp: impression, track: true }, { imp: impression2 }, { imp: impression3 }]);

const lastArgs = fakeImpressionsCache.track.mock.calls[fakeImpressionsCache.track.mock.calls.length - 1];

Expand Down
2 changes: 1 addition & 1 deletion src/trackers/eventTracker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export function eventTrackerFactory(
whenInit(() => {
// Wrap in a timeout because we don't want it to be blocking.
setTimeout(() => {
// copy of event, to avoid unexpected behaviour if modified by integrations
// copy of event, to avoid unexpected behavior if modified by integrations
const eventDataCopy = objectAssign({}, eventData);
if (properties) eventDataCopy.properties = objectAssign({}, properties);
// integrationsManager does not throw errors (they are internally handled by each integration module)
Expand Down
2 changes: 1 addition & 1 deletion src/trackers/impressionsTracker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export function impressionsTrackerFactory(
if (impressionListener || integrationsManager) {
for (let i = 0; i < impressionsLength; i++) {
const impressionData: SplitIO.ImpressionData = {
// copy of impression, to avoid unexpected behaviour if modified by integrations or impressionListener
// copy of impression, to avoid unexpected behavior if modified by integrations or impressionListener
impression: objectAssign({}, impressions[i].imp),
attributes,
ip,
Expand Down
2 changes: 1 addition & 1 deletion src/utils/settingsValidation/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ export function settingsValidation(config: unknown, validationParams: ISettingsV
if (withDefaults.mode === LOCALHOST_MODE && maybeKey === undefined) {
withDefaults.core.key = 'localhost_key';
} else {
// Keeping same behaviour than JS SDK: if settings key or TT are invalid,
// Keeping same behavior than JS SDK: if settings key or TT are invalid,
// `false` value is used as bound key/TT of the default client, which leads to some issues.
// @ts-ignore, @TODO handle invalid keys as a non-recoverable error?
withDefaults.core.key = validateKey(log, maybeKey, LOG_PREFIX_CLIENT_INSTANTIATION);
Expand Down

0 comments on commit 045e38b

Please sign in to comment.