feat: server-side contract tests #197

cwaldren-ld · 2023-08-16T19:39:53Z

Introduces a contract test server for the server-side SDK, runs in CI, and initial (large) suppression list.

The bulk of the code is copied from the client-side, although I have made an effort to refactor the common JSON definitions into a shared library.

There is an opportunity to refactor out the "shell" (the coroutine stuff that is responsible for serving/responding to requests) into a common lib as well, but it didn't seem worth the effort.

shortcut-integration · 2023-08-16T19:39:55Z

This pull request has been linked to Shortcut Story #206687: Contract test support.

cwaldren-ld · 2023-08-18T01:14:42Z

.github/workflows/server.yml

@@ -6,11 +6,30 @@ on:
    paths-ignore:
      - '**.md' #Do not need to run CI for markdown changes.
  pull_request:
-    branches: [ main, server-side ]
+    branches: [ main, server-side, cw/sc-206686/server-build-ci ]


TODO: remove. Needed to run CI since I rebased this PR from server-side to the server-build-ci branch.

cwaldren-ld · 2023-08-21T17:00:18Z

contract-tests/server-contract-tests/test-suppressions.txt

@@ -0,0 +1,874 @@
+evaluation/parameterized/bad attribute reference errors - tilde followed by nothing/test-flag/evaluate flag with detail


A huge amount of these are all flags related, since the test harness uses that method to make assertions. I've fixed that separately in #201.

cwaldren-ld · 2023-08-21T17:03:45Z

libs/common/include/launchdarkly/data/evaluation_detail.hpp

+     * @return True if a reason exists and matches the given kind.
+     */
+    [[nodiscard]] bool ReasonKindIs(enum EvaluationReason::Kind kind) const;
+


I found myself needing to reach into the reason and check if it was a specific kind, like:

if (detail.Reason() && detail.Reason()->Kind() == EvaluationReason::Kind::kWhatever)

I feel like this could be a reasonably common task and deserves a method, but perhaps not.

I suppose it doesn't hurt anything.

cwaldren-ld · 2023-08-21T17:04:26Z

libs/internal/include/launchdarkly/data_model/flag.hpp

@@ -88,7 +88,7 @@ struct Flag {
    std::vector<Target> targets;
    std::vector<Target> contextTargets;
    std::vector<Rule> rules;
-    std::optional<std::uint64_t> offVariation;
+    std::optional<std::int64_t> offVariation;


Our JSON specification notes that the minimum values are 0, so I used uint64_t - but that's not the case in practice, as the test harness may send negative values.

cwaldren-ld · 2023-08-21T17:05:10Z

libs/internal/src/events/asio_event_processor.cpp

+
+                // Object slicing on purpose; the context will be stripped out
+                // of the ServerTrackEventParams when converted to a
+                // TrackEventParams.
                out.emplace_back(std::move(event));


Could maybe solve this with composition instead.

cwaldren-ld · 2023-08-21T17:07:41Z

libs/server-sdk/src/data_sources/data_source_event_handler.cpp

@@ -62,7 +62,7 @@ tl::expected<std::optional<DataSourceEventHandler::Put>, JsonError> tag_invoke(
    auto const& obj = json_value.as_object();
    PARSE_FIELD(path, obj, "path");
    // We don't know what to do with a path other than "/".
-    if (path != "/") {
+    if (!(path == "/" || path.empty())) {


The path may be omitted in PUT events.

cwaldren-ld · 2023-08-21T17:07:56Z

libs/server-sdk/src/evaluation/evaluator.cpp

@@ -138,7 +138,7 @@ EvaluationDetail<Value> Evaluator::FlagVariation(
    Flag const& flag,
    Flag::Variation variation_index,
    EvaluationReason reason) const {
-    if (variation_index >= flag.variations.size()) {
+    if (variation_index < 0 || variation_index >= flag.variations.size()) {


Now that variation_index is signed, must check for negative.

cwaldren-ld · 2023-08-23T20:06:54Z

libs/server-sdk/src/client_impl.cpp

@@ -200,7 +203,21 @@ template <typename T>
 EvaluationDetail<T> ClientImpl::VariationInternal(Context const& ctx,


All the grossness in this method that follows is fixed in a future PR (#203)

libs/server-sdk/src/client_impl.cpp

Implements the `AllFlagsState` API, which evaluates all flags and returns a serializable structure suitable for bootstrapping a client-side SDK. The method returns an `AllFlagsState` class, which has a nested `State` class representing details for a single flag. Additionally, there is an `Options` enum to control the API's behavior.

…tive variations

cwaldren-ld changed the title ~~feat: server-side contract tests [draft\~~ feat: server-side contract tests [draft] Aug 16, 2023

cwaldren-ld force-pushed the cw/sc-206687/contract-tests branch from ae5deb7 to 07e31d1 Compare August 17, 2023 18:55

cwaldren-ld changed the base branch from server-side to cw/sc-206686/server-build-ci August 17, 2023 18:55

cwaldren-ld commented Aug 18, 2023

View reviewed changes

Base automatically changed from cw/sc-206686/server-build-ci to server-side August 21, 2023 16:50

cwaldren-ld force-pushed the cw/sc-206687/contract-tests branch from 4a8eac0 to f115ebb Compare August 21, 2023 16:53

cwaldren-ld added 20 commits August 21, 2023 09:54

add <array> header

607024a

add another missing <array>

f49d534

rename sdk-contract-tests to client-contract-tests

53c8c54

copy client-side contract test server to server-side

d14350b

refactor contract tests data model into dedicated library

f1ebea7

fix client name and version in contract test binaries

aa85920

update evaluation/evaluation detail commands to deserialize contexts

709c995

re-enable client-entity

5406b59

value-init data model types during deserialization

e45741b

add server Github action workflow

298c6d8

update client Github action workflow to fix contract test binary path

d1976df

be accepting of an empty PUT path

dbb541d

more suppressions

0b3f00a

fix all custom event contract tests

ac96b10

attempt to make CI run

2d4e5b8

fix some event tests

0829bca

remove a bunch of suppressions

e2d01df

add missing version to feature events

37e604a

fix debug event tests

22911a2

fix some feature event emission

2ddf672

cwaldren-ld force-pushed the cw/sc-206687/contract-tests branch from f115ebb to 2ddf672 Compare August 21, 2023 16:55

remove old PR branch from server.yml

a41423f

cwaldren-ld commented Aug 21, 2023

View reviewed changes

cwaldren-ld added 2 commits August 21, 2023 10:08

update comment

d5ce5af

additional suppressions for bucketing

8a83ad6

cwaldren-ld marked this pull request as ready for review August 22, 2023 19:54

cwaldren-ld requested review from a team and kinyoklion August 22, 2023 19:54

cwaldren-ld changed the title ~~feat: server-side contract tests [draft]~~ feat: server-side contract tests Aug 22, 2023

cwaldren-ld commented Aug 23, 2023

View reviewed changes

kinyoklion approved these changes Aug 23, 2023

View reviewed changes

libs/server-sdk/src/client_impl.cpp Show resolved Hide resolved

cwaldren-ld added 2 commits August 23, 2023 14:10

update data model deserialization tests to reflect acceptance of nega…

cec8d4b

…tive variations

cwaldren-ld merged commit 9b38361 into server-side Aug 23, 2023
12 checks passed

cwaldren-ld deleted the cw/sc-206687/contract-tests branch August 23, 2023 21:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: server-side contract tests #197

feat: server-side contract tests #197

cwaldren-ld commented Aug 16, 2023 •

edited

Loading

shortcut-integration bot commented Aug 16, 2023

cwaldren-ld Aug 18, 2023

cwaldren-ld Aug 21, 2023 •

edited

Loading

cwaldren-ld Aug 21, 2023

kinyoklion Aug 23, 2023

cwaldren-ld Aug 21, 2023

cwaldren-ld Aug 21, 2023 •

edited

Loading

cwaldren-ld Aug 21, 2023

cwaldren-ld Aug 21, 2023

cwaldren-ld Aug 23, 2023

		@@ -0,0 +1,874 @@
		evaluation/parameterized/bad attribute reference errors - tilde followed by nothing/test-flag/evaluate flag with detail

		@@ -200,7 +203,21 @@ template <typename T>
		EvaluationDetail<T> ClientImpl::VariationInternal(Context const& ctx,

feat: server-side contract tests #197

feat: server-side contract tests #197

Conversation

cwaldren-ld commented Aug 16, 2023 • edited Loading

shortcut-integration bot commented Aug 16, 2023

cwaldren-ld Aug 18, 2023

Choose a reason for hiding this comment

cwaldren-ld Aug 21, 2023 • edited Loading

Choose a reason for hiding this comment

cwaldren-ld Aug 21, 2023

Choose a reason for hiding this comment

kinyoklion Aug 23, 2023

Choose a reason for hiding this comment

cwaldren-ld Aug 21, 2023

Choose a reason for hiding this comment

cwaldren-ld Aug 21, 2023 • edited Loading

Choose a reason for hiding this comment

cwaldren-ld Aug 21, 2023

Choose a reason for hiding this comment

cwaldren-ld Aug 21, 2023

Choose a reason for hiding this comment

cwaldren-ld Aug 23, 2023

Choose a reason for hiding this comment

cwaldren-ld commented Aug 16, 2023 •

edited

Loading

cwaldren-ld Aug 21, 2023 •

edited

Loading

cwaldren-ld Aug 21, 2023 •

edited

Loading