chore: Update data sources to send FDv2-compatible payloads #229
Conversation
This initial commit is quite limited in scope. It updates the existing server-side contract tests to send the FDv2 equivalent payloads to SDKs under tests. These changes do not include **any new tests**. Additional commits are required to expand test coverage to include FDv2 specific handling (e.g. xfer-changes, state handling). It should also be noted these changes have likely broken the client, mobile, PHP, and Roku integrations. That's okay because we will be updating each of those in subsequent commits as well as the FDv2 work progresses.
keelerm84
left a comment
keelerm84
left a comment
There was a problem hiding this comment.
This is a VERY rough first pass at getting the test harness working with FDv2. Lots of todos, but I figure it is better to start getting stuff reviewed and merged so it is accessible to both of us.
| @@ -0,0 +1,68 @@ | |||
| package framework | |||
There was a problem hiding this comment.
Same basic types I'm carrying around everywhere right now. Need to get these into the events package or somewhere core.
| func (p *PollingService) standardPollingHandler() http.Handler { | ||
| return p.pollingHandler(func(p *PollingService, r *http.Request) []byte { | ||
| return p.currentData.Serialize() | ||
| fdv2SdkData, ok := p.currentData.(FDv2SDKData) |
There was a problem hiding this comment.
This translation bit is a mess and is going to require the most rework. I wanted to see how the changes emerged as we developed additional FDv2 tests (like the xfer-changes stuff) before I tried to fix this hack too much.
| return p.pollingHandler(func(p *PollingService, r *http.Request) []byte { | ||
| data, _ := p.currentData.(ServerSDKData) | ||
| return data["flags"][mux.Vars(r)["key"]] | ||
| // TODO: Update this logic |
There was a problem hiding this comment.
PHP stuff will come later.
| "/sdk/latest-all", | ||
| ) | ||
| } | ||
| // TODO: Re-enable these tests |
There was a problem hiding this comment.
I will fix the unit tests later. Again, just getting something working out in front of you.
| Serialize() []byte | ||
| } | ||
|
|
||
| type FDv2SDKData []framework.BaseObject |
There was a problem hiding this comment.
This type may have to be more complex over time. We might need something to control the state, the version, and potential the server intent. That way the serialize method can render the full payload and we don't have to do that hacky translation I called out earlier.
mockld/sdk_data.go
Outdated
| // We use this for both regular server-side SDKs and the PHP SDK. | ||
| type ServerSDKData map[DataItemKind]map[string]json.RawMessage | ||
|
|
||
| func (s ServerSDKData) AsFDv2SDKData(t *ldtest.T) FDv2SDKData { |
There was a problem hiding this comment.
The data model in the parameterized tests used to match the SDK format. That doesn't work anymore. For now, I do a simple translation. Ideally we would do this when loading the files.
| // how we set up the polling endpoints in the mock data source, and that is handled | ||
| // transparently by the logic in mockld/polling_service.go based on the fact that the | ||
| // "SDK kind" configured for test suite is PHP. | ||
| t.Run("parameterized", runParameterizedServerSideEvalTests) |
There was a problem hiding this comment.
Do these deserve TODOs? I feel like it could get lost.
There was a problem hiding this comment.
I can actually add this back in, so we are good there.
sdktests/testapi_sdk_data.go
Outdated
| } | ||
|
|
||
| t.Debug("setting SDK data to: %s", string(data.Serialize())) | ||
| jsonStr, _ := json.Marshal(data) |
There was a problem hiding this comment.
could check the error, then use t to assert it is nil
keelerm84
force-pushed
the
mk/sc-254960/fdv2
branch
2 times, most recently
from
888419c to
1720df1
Compare
keelerm84
force-pushed
the
mk/sc-254960/fdv2
branch
from
1720df1 to
5809c1b
Compare
This initial commit is quite limited in scope. It updates the existing server-side contract tests to send the FDv2 equivalent payloads to SDKs under tests. These changes do not include **any new tests**. Additional commits are required to expand test coverage to include FDv2 specific handling (e.g. xfer-changes, state handling). It should also be noted these changes have likely broken the client, mobile, PHP, and Roku integrations. That's okay because we will be updating each of those in subsequent commits as well as the FDv2 work progresses.
This initial commit is quite limited in scope. It updates the existing server-side contract tests to send the FDv2 equivalent payloads to SDKs under tests. These changes do not include **any new tests**. Additional commits are required to expand test coverage to include FDv2 specific handling (e.g. xfer-changes, state handling). It should also be noted these changes have likely broken the client, mobile, PHP, and Roku integrations. That's okay because we will be updating each of those in subsequent commits as well as the FDv2 work progresses.
This initial commit is quite limited in scope. It updates the existing server-side contract tests to send the FDv2 equivalent payloads to SDKs under tests.
These changes do not include any new tests. Additional commits are required to expand test coverage to include FDv2 specific handling (e.g. xfer-changes, state handling).
It should also be noted these changes have likely broken the client, mobile, PHP, and Roku integrations. That's okay because we will be updating each of those in subsequent commits as well as the FDv2 work progresses.