Fix more AllFlagsState contract tests & much needed unit test refactoring (#213)

* Fix a couple more AllFlagsState contract tests (excluding experimentation test)

* Refactor AllFlagsState tests to stop using internal interfaces, add comments.

Author: cwaldren-ld

src/all_flags_state.c

@@ -61,6 +61,7 @@ LDi_newFlagState(const char* key) {
     state->debugEventsUntilDate = 0;
     state->trackEvents = LDBooleanFalse;
     state->version = 0;
+    state->omitDetails = LDBooleanFalse;
 
     return state;
 }
@@ -261,13 +262,15 @@ static LDBoolean buildFlagState(const struct LDAllFlagsState *const flags, struc
         }
 
 
-        if (!LDObjectSetNumber(flagObj, "version", flag->version)) {
-            LD_LOG(LD_LOG_ERROR, "set key");
+        if (!flag->omitDetails) {
+            if (!LDObjectSetNumber(flagObj, "version", flag->version)) {
+                LD_LOG(LD_LOG_ERROR, "set key");
 
-            return LDBooleanFalse;
+                return LDBooleanFalse;
+            }
         }
 
-        if (flag->details.reason != LD_UNKNOWN) {
+        if (flag->details.reason != LD_UNKNOWN && !flag->omitDetails) {
             struct LDJSON* reason = NULL;
 
             if (!(reason = LDReasonToJSON(&flag->details))) {
@@ -370,22 +373,19 @@ LDi_freeAllFlagsBuilder(struct LDAllFlagsBuilder *const builder) {
 LDBoolean
 LDi_allFlagsBuilderAdd(struct LDAllFlagsBuilder *const b, struct LDFlagState *const flag)
 {
-    LDBoolean wantReason;
 
     LD_ASSERT(b);
 
-    wantReason = b->includeReasons;
-
-    if (wantReason && b->detailsOnlyForTrackedFlags) {
+    if (b->detailsOnlyForTrackedFlags) {
         double now;
 
         LDi_getMonotonicMilliseconds(&now);
         if (!flag->trackEvents && !(flag->debugEventsUntilDate != 0 && flag->debugEventsUntilDate > now)) {
-            wantReason = LDBooleanFalse;
+            flag->omitDetails = LDBooleanTrue;
         }
     }
 
-    if (!wantReason) {
+    if (!b->includeReasons) {
         if (flag->details.reason == LD_RULE_MATCH) {
             LDFree(flag->details.extra.rule.id);
         } else if (flag->details.reason == LD_PREREQUISITE_FAILED) {

src/all_flags_state.h

@@ -38,6 +38,10 @@ struct LDFlagState
     LDBoolean trackEvents;
 
     double debugEventsUntilDate;
+
+    /* omitDetails is true if, based on the options passed to AllFlagsState and the flag state, some of the
+     * metadata can be left out of the JSON representation. */
+    LDBoolean omitDetails;
 };
 
 /** @brief Returns a pointer to an `LDAllFlagsState` with specified validity.

src/evaluate.c

@@ -203,6 +203,7 @@ static LDBoolean  addValue(
         *result               = NULL;
         details->hasVariation = LDBooleanFalse;
         details->reason = LD_ERROR;
+        details->extra.errorKind = LD_MALFORMED_FLAG;
 
         return LDBooleanFalse;
     }

src/variations.c

@@ -827,17 +827,14 @@ LDAllFlagsState(struct LDClient *const client, const struct LDUser *const user,
 
         LDJSONFree(eventsUnused);
 
-        if (flag->value) {
-            LDi_flagModelPopulate(&model, flag);
+        LDi_flagModelPopulate(&model, flag);
 
-            if (!LDi_allFlagsBuilderAdd(builder, flag)) {
-                LDi_freeFlagState(flag);
-
-                goto cleanup;
-            }
-        } else {
+        if (!LDi_allFlagsBuilderAdd(builder, flag)) {
             LDi_freeFlagState(flag);
+
+            goto cleanup;
         }
+
     }
 
     success = LDBooleanTrue;

test-utils/include/test-utils/flags.h

@@ -15,6 +15,9 @@ setFallthrough(struct LDJSON *const flag, const unsigned int variation);
 void
 addVariation(struct LDJSON *const flag, struct LDJSON *const variation);
 
+void
+addPrerequisite(struct LDJSON *const flag, struct LDJSON *const prereq, unsigned int expectedPrereqVariation);
+
 void
 addVariations1(struct LDJSON *const flag);
 

test-utils/src/flags.c

@@ -44,6 +44,23 @@ addVariation(struct LDJSON *const flag, struct LDJSON *const variation)
     LD_ASSERT(LDArrayPush(variations, variation));
 }
 
+void
+addPrerequisite(struct LDJSON *const flag, struct LDJSON *const prereq, unsigned int expectedVariation)
+{
+    struct LDJSON *req;
+    struct LDJSON *reqs;
+
+    if (!(reqs = LDObjectLookup(flag, "prerequisites"))) {
+        reqs = LDNewArray();
+        LDObjectSetKey(flag, "prerequisites", reqs);
+    }
+
+    req = LDNewObject();
+    LDObjectSetKey(req, "key", LDNewText(LDGetText(LDObjectLookup(prereq, "key"))));
+    LDObjectSetKey(req, "variation", LDNewNumber(expectedVariation));
+    LDArrayPush(reqs, req);
+}
+
 void
 addVariations1(struct LDJSON *const flag)
 {

testharness-suppressions.txt

@@ -3,9 +3,6 @@ evaluation/bucketing/bucket by non-key attribute/rollouts/invalid value type
 evaluation/bucketing/bucket by non-key attribute/experiments/integer value
 evaluation/bucketing/bucket by non-key attribute/experiments/invalid value type
 evaluation/all flags state/experimentation
-evaluation/all flags state/error in flag/without reasons
-evaluation/all flags state/error in flag/with reasons
-evaluation/all flags state/details only for tracked flags
 events/feature events/full feature event for tracked flag/without reason/non-anonymous user/valid flag/type: bool
 events/feature events/full feature event for tracked flag/without reason/non-anonymous user/valid flag/type: int
 events/feature events/full feature event for tracked flag/without reason/non-anonymous user/valid flag/type: double