Attributes binding evaluation - #68
Merged
Merged
Conversation
emmaz90
force-pushed
the
attributes_binding_evaluation
branch
from
January 11, 2022 20:20
01ddeef to
d7f908a
Compare
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
EmilianoSanchez
requested changes
Jan 17, 2022
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); |
Contributor
There was a problem hiding this comment.
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);
EmilianoSanchez
requested changes
Jan 17, 2022
| @@ -19,25 +19,33 @@ export function clientAttributesDecoration<TClient extends SplitIO.IClient | Spl | |||
| const clientTrack = client.track; | |||
|
|
|||
Contributor
There was a problem hiding this comment.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Javascript commons library
What did you accomplish?
Add attributes binding evaluation logic
How do we test the changes introduced in this PR?
Extra Notes