Skip to content

Attributes binding evaluation - #68

Merged
emmaz90 merged 6 commits into
attributes_bindingfrom
attributes_binding_evaluation
Jan 17, 2022
Merged

Attributes binding evaluation#68
emmaz90 merged 6 commits into
attributes_bindingfrom
attributes_binding_evaluation

Conversation

@emmaz90

@emmaz90 emmaz90 commented Jan 11, 2022

Copy link
Copy Markdown
Contributor

Javascript commons library

What did you accomplish?

Add attributes binding evaluation logic

How do we test the changes introduced in this PR?

Extra Notes

@emmaz90
emmaz90 force-pushed the attributes_binding_evaluation branch from 01ddeef to d7f908a Compare January 11, 2022 20:20
@emmaz90
emmaz90 changed the base branch from attributes_binding to attributes_binding_storage January 13, 2022 15:38
Base automatically changed from attributes_binding_storage to attributes_binding January 14, 2022 20:37
Comment on lines +1 to +79
import { sdkClientMethodCSFactory } from '../sdkClientMethodCS';

import { settingsWithKey } from '../../utils/settingsValidation/__tests__/settings.mocks';

const partialStorages: { destroy: jest.Mock }[] = [];

const storageMock = {
destroy: jest.fn(),
shared: jest.fn(() => {
partialStorages.push({ destroy: jest.fn() });
return partialStorages[partialStorages.length - 1];
})
};

const partialSdkReadinessManagers: { sdkStatus: jest.Mock, readinessManager: { destroy: jest.Mock } }[] = [];

const sdkReadinessManagerMock = {
sdkStatus: jest.fn(),
readinessManager: {
destroy: jest.fn(),
isDestroyed: jest.fn(() => { return false; }),
isReady: jest.fn(() => { return true; })
},
shared: jest.fn(() => {
partialSdkReadinessManagers.push({
sdkStatus: jest.fn(),
readinessManager: { destroy: jest.fn() },
});
return partialSdkReadinessManagers[partialSdkReadinessManagers.length - 1];
})
};

const partialSyncManagers: { start: jest.Mock, stop: jest.Mock, flush: jest.Mock }[] = [];

const syncManagerMock = {
stop: jest.fn(),
flush: jest.fn(() => Promise.resolve()),
shared: jest.fn(() => {
partialSyncManagers.push({ start: jest.fn(), stop: jest.fn(), flush: jest.fn(() => Promise.resolve()) });
return partialSyncManagers[partialSyncManagers.length - 1];
})
};

const params = {
storage: storageMock,
sdkReadinessManager: sdkReadinessManagerMock,
syncManager: syncManagerMock,
signalListener: { stop: jest.fn() },
settings: settingsWithKey
};

// We are testing attributes binding feature and we just need to know how the attributes are combined before evaluation
// So input validation decorator is mocked to return the combined attributes instead of evaluation so we can verify them
jest.mock('../clientInputValidation', () => {
return {
clientInputValidationDecorator() {
return {
getTreatment(maybeKey: any, maybeSplit: string, maybeAttributes?: any) {
return maybeAttributes;
},
getTreatmentWithConfig(maybeKey: any, maybeSplit: string, maybeAttributes?: any) {
return maybeAttributes;
},
getTreatments(maybeKey: any, maybeSplits: string[], maybeAttributes?: any) {
return maybeAttributes;
},
getTreatmentsWithConfig(maybeKey: any, maybeSplits: string[], maybeAttributes?: any) {
return maybeAttributes;
}
};
}
};
});

// @ts-expect-error
const sdkClientMethod = sdkClientMethodCSFactory(params);
const client = sdkClientMethod();

//expect(client).toBe(AttributesDecorationMockedClient);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since it is a UT for clientAttributesDecoration, it is better to test it calling the clientAttributesDecoration function directly.
So, replace all lines above, with something like the following:

import { clientAttributesDecoration } from '../clientAttributesDecoration';
import { loggerMock } from '../../logger/__tests__/sdkLogger.mock';

// mocked methods return the provided attributes object (2nd argument), to assert that it was properly passed
const clientMock = {
  getTreatment(maybeKey: any, maybeSplit: string, maybeAttributes?: any) {
    return maybeAttributes;
  },
  getTreatmentWithConfig(maybeKey: any, maybeSplit: string, maybeAttributes?: any) {
    return maybeAttributes;
  },
  getTreatments(maybeKey: any, maybeSplits: string[], maybeAttributes?: any) {
    return maybeAttributes;
  },
  getTreatmentsWithConfig(maybeKey: any, maybeSplits: string[], maybeAttributes?: any) {
    return maybeAttributes;
  }
};
// @ts-expect-error
const client = clientAttributesDecoration(loggerMock, clientMock);

@@ -19,25 +19,33 @@ export function clientAttributesDecoration<TClient extends SplitIO.IClient | Spl
const clientTrack = client.track;

@EmilianoSanchez EmilianoSanchez Jan 17, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

in line 10, please remove the any as return type, so that TS automatically infers it for our UTs and the rest of the code.

@EmilianoSanchez EmilianoSanchez left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@emmaz90
emmaz90 merged commit f9efa55 into attributes_binding Jan 17, 2022
@emmaz90
emmaz90 deleted the attributes_binding_evaluation branch January 17, 2022 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants