Implement the test data source.#27
Conversation
|
This pull request has been linked to Shortcut Story #154356: Implement test data source.. |
| td.update(flag); | ||
| const flagCopy = td.flag('test-flag'); | ||
|
|
||
| const flagRules: FlagRule[] = [{ |
There was a problem hiding this comment.
So, being able to have this FlagRule[] here prevents some of the problems with the original testing.
In the original versions of these tests the operator is also in the test, versus op. So everything seems fine.
|
|
||
| it('can set a value for all', () => { | ||
| const flag = td.flag('test-flag'); | ||
| flag.valueForAll('potato'); |
There was a problem hiding this comment.
Not tested in original tests.
| expect(flagCopy).not.toEqual(flag); | ||
| }); | ||
|
|
||
| it('can clone a complex flag configuration', () => { |
There was a problem hiding this comment.
Copying a complex object was not a consideration initially. Because the flag configuration was just data, it didn't have things like AttributeReferences in it.
| @@ -0,0 +1,398 @@ | |||
| import { AttributeReference, LDClientImpl } from '../../../src'; | |||
There was a problem hiding this comment.
The original version of these tests worked with the client, this variation just uses a store. I don't think using fewer dependencies is a bad change in this case.
There was a problem hiding this comment.
Longer term I can test things more by updating some of the other tests to use this instead of hand constructed objects. I will use it for the client level tests as well of course.
| @@ -0,0 +1,194 @@ | |||
| import { LDStreamProcessor } from '../../api'; | |||
There was a problem hiding this comment.
I've not exported TestData yet. I put it in this integrations folder, and need to work on how the exports will work.
| * Finally, call `thenReturn` to finish defining the rule. | ||
| */ | ||
|
|
||
| export default class TestDataRuleBuilder<BuilderType> { |
There was a problem hiding this comment.
This is the type I made generic to remove the circular dependency.
| variation?: number, | ||
| ) { | ||
| if (clauses) { | ||
| this.clauses = JSON.parse(JSON.stringify(clauses)); |
There was a problem hiding this comment.
I use JSON cloning a few places in here. Being as this is for specific testing situations I cannot imagine it being too much of a problem.
There was a problem hiding this comment.
Why wouldn't a shallow copy of the array work? We won't be modifying anything inside of an individual clause.
There was a problem hiding this comment.
I've made this change.
|
|
||
| it('can make not matching rules', () => { | ||
| const flag = td.flag('flag') | ||
| .ifNotMatch('user', 'name', 'Saffron', 'Bubble') |
There was a problem hiding this comment.
This was not tested in the original.
| this.tags = new ApplicationTags(validatedOptions); | ||
| this.diagnosticRecordingInterval = validatedOptions.diagnosticRecordingInterval; | ||
|
|
||
| if (TypeValidators.Function.is(validatedOptions.featureStore)) { |
There was a problem hiding this comment.
So, I am using the validated features for this. Which is a little different than LDOptions. So I may need to make some updates when I handle the updates for integrations like redis.
There was a problem hiding this comment.
It moved down here, so the rest of config can be initialized before using it. Could defer construction. A subset of the configuration interface, with specifically what stores need, is probably better.
| } | ||
|
|
||
| function processFlag(flag: Flag) { | ||
| /** |
There was a problem hiding this comment.
Exported so they can be used handling the free-form flag data.
This is why in other SDKs like Go, Java, and .NET, the idea of a data source factory is represented as an interface with a particular method, rather than just being a function: because it's often advantageous for the same class to be able to do more than one thing. (As another example, the Redis and DynamoDB data source builders in those SDKs are usable as both a regular data store factory and a big segment store factory.) |
2 participants
There are a few things that are different from this and the node implementation.
Unlike the other code that has been ported I have removed the interface files. This is because there are cross dependencies between the different components of the implementation and all the dependencies between components cannot be represented by the interfaces. We need methods that are implementation only and not intended for external consumption.
One of the implementations is also a generic to allow having the implementation, with cross dependencies, span multiple files. If it was not this way, then there would be a circular dependency between the types.
An additional change is that constructing a test data object returns an actual test data object instead of a function/object that is a factory for data sources as well as being the implementation for the TestData object. Instead you call a method to get a factory, and that is given to the configuration.