test: improve gherkin test suite only relying on the newest version, with data loading #1578

aepfli · 2025-08-25T06:26:24Z

based on: open-feature/spec#336
depends on: open-feature/spec#336

gemini-code-assist

Summary of Changes

Hello @aepfli, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly refactors and enhances the Gherkin test suite to align with the latest OpenFeature specification, particularly regarding flag evaluation and context handling. It transitions from hardcoded test flags to dynamic loading from a JSON file, introduces a Common Expression Language (CEL) based context evaluator for advanced targeting, and expands the Gherkin step definitions to support more complex context scenarios and detailed evaluation result assertions.

Highlights

Dynamic Test Flag Loading: Test flags are now loaded from 'test-flags.json' using Jackson, enabling more flexible and spec-aligned test data, and replacing previous hardcoded flag definitions.
CEL-based Context Evaluation: A new 'CelContextEvaluator' is introduced, allowing for sophisticated flag targeting logic within tests using Common Expression Language (CEL).
Enhanced Gherkin Steps: New Gherkin steps are added to define contexts with null values, specific types, targeting keys, and nested properties, as well as to assert flag keys, error codes, and error messages in evaluation results.
Specification Update: The OpenFeature specification submodule has been updated, reflecting changes that necessitate these testing improvements and ensuring alignment with the latest spec.
Improved Flag Evaluation Logic: The 'InMemoryProvider' now handles 'ContextEvaluator' failures gracefully by falling back to the default variant and correctly setting the evaluation reason, improving robustness.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request updates the Gherkin test suite to align with a newer version of the OpenFeature specification, notably by loading test flags from a JSON file instead of defining them in code. The changes are extensive, introducing Jackson for deserialization and the CEL library for context evaluation in tests.

My review focuses on improving robustness and efficiency. I've identified a potential issue in the InMemoryProvider where exceptions are swallowed, which could hide bugs. I've also suggested performance improvements for the new CelContextEvaluator and pointed out some dead/duplicated code in the test utilities that could be cleaned up.

src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java

src/test/java/dev/openfeature/sdk/testutils/TestFlagsUtils.java

src/test/java/dev/openfeature/sdk/testutils/jackson/CelContextEvaluator.java

src/test/java/dev/openfeature/sdk/testutils/jackson/ImmutableMetadataMixin.java

aepfli · 2025-08-25T07:31:43Z

/gemini review

…with data loading Signed-off-by: Simon Schrottner <[email protected]>

gemini-code-assist

Code Review

This pull request is a significant improvement, aligning the Gherkin test suite with the latest OpenFeature specification by loading test data from JSON and using CEL for dynamic evaluation. The changes are well-structured and greatly enhance test maintainability. I've identified a few areas for improvement, including logging a swallowed exception in the InMemoryProvider, fixing a bug in a Gherkin step definition, and improving the robustness of the new test utility classes. I've provided detailed suggestions for each point. Overall, this is an excellent enhancement to the SDK's testing framework.

src/test/java/dev/openfeature/sdk/testutils/jackson/VariantsMapDeserializer.java

src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java

src/test/java/dev/openfeature/sdk/e2e/steps/ContextSteps.java

src/test/java/dev/openfeature/sdk/testutils/jackson/CelContextEvaluator.java

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Simon Schrottner <[email protected]>

…Evaluator.java Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Simon Schrottner <[email protected]>

@given

…with data loading Signed-off-by: Simon Schrottner <[email protected]> diff --git c/src/test/java/dev/openfeature/sdk/e2e/steps/ContextSteps.java i/src/test/java/dev/openfeature/sdk/e2e/steps/ContextSteps.java index e4cd603..121b567 100644 --- c/src/test/java/dev/openfeature/sdk/e2e/steps/ContextSteps.java +++ i/src/test/java/dev/openfeature/sdk/e2e/steps/ContextSteps.java @@ -114,9 +114,9 @@ public class ContextSteps { public void a_context_containing_a_key_with_type_and_with_value(String key, String type, String value) throws ClassNotFoundException, InstantiationException { Map<String, Value> map = state.context.asMap(); - Map<String, Value> map = state.context.asMap(); map.put(key, new Value(Utils.convert(value, type))); state.context = new MutableContext(state.context.getTargetingKey(), map); + } @given("a context containing a targeting key with value {string}") public void a_context_containing_a_targeting_key_with_value(String string) { diff --git c/src/test/java/dev/openfeature/sdk/testutils/jackson/CelContextEvaluator.java i/src/test/java/dev/openfeature/sdk/testutils/jackson/CelContextEvaluator.java index 20f3f5f..138c23f 100644 --- c/src/test/java/dev/openfeature/sdk/testutils/jackson/CelContextEvaluator.java +++ i/src/test/java/dev/openfeature/sdk/testutils/jackson/CelContextEvaluator.java @@ -46,7 +46,6 @@ public class CelContextEvaluator<T> implements ContextEvaluator<T> { // Evaluate with context, overriding defaults. objectMap.putAll(evaluationContext.asObjectMap()); } - } Object result = program.eval(objectMap); diff --git c/src/test/java/dev/openfeature/sdk/testutils/jackson/VariantsMapDeserializer.java i/src/test/java/dev/openfeature/sdk/testutils/jackson/VariantsMapDeserializer.java index 339d8c8..9a63f58 100644 --- c/src/test/java/dev/openfeature/sdk/testutils/jackson/VariantsMapDeserializer.java +++ i/src/test/java/dev/openfeature/sdk/testutils/jackson/VariantsMapDeserializer.java @@ -50,6 +50,7 @@ public class VariantsMapDeserializer extends JsonDeserializer<Map<String, Object } private Object convertJsonNodeToValue(JsonNode node) throws InstantiationException { + if (node.isNull()) { return null; } else if (node.isBoolean()) { @@ -60,25 +61,8 @@ public class VariantsMapDeserializer extends JsonDeserializer<Map<String, Object return node.asDouble(); } else if (node.isTextual()) { return node.asText(); - } else if (node.isArray()) { - List<Object> list = new ArrayList<>(); - for (JsonNode item : node) { - list.add(convertJsonNodeToValue(item)); - } - return list; } else if (node.isObject()) { - Map<String, Value> map = new HashMap<>(); - Iterator<Map.Entry<String, JsonNode>> fields = node.fields(); - while (fields.hasNext()) { - Map.Entry<String, JsonNode> field = fields.next(); - Object o = convertJsonNodeToValue(field.getValue()); - if (o instanceof Value) { - map.put(field.getKey(), (Value) o); - } else { - map.put(field.getKey(), new Value(o)); - } - } - return new Value(new MutableStructure(map)); + return Value.objectToValue(node); } throw new IllegalArgumentException("Unsupported JSON node type: " + node.getNodeType()); Signed-off-by: Simon Schrottner <[email protected]>

aepfli · 2025-08-25T11:26:32Z

/gemini review

gemini-code-assist

Code Review

This pull request improves the Gherkin test suite by loading test data from JSON files, aligning with the latest OpenFeature specification. The changes are well-structured, introducing custom Jackson deserializers and a CEL-based context evaluator for dynamic flag evaluation. My review includes suggestions to improve dependency management in pom.xml, enhance thread-safety in test utilities, improve exception handling, and increase code clarity in test steps.

src/test/java/dev/openfeature/sdk/testutils/TestFlagsUtils.java

pom.xml

src/main/java/dev/openfeature/sdk/providers/memory/InMemoryProvider.java

src/test/java/dev/openfeature/sdk/e2e/Utils.java

src/test/java/dev/openfeature/sdk/e2e/steps/FlagStepDefinitions.java

codecov · 2025-08-25T11:30:41Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.16%. Comparing base (5474c73) to head (af30a35).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1578      +/-   ##
============================================
+ Coverage     92.82%   93.16%   +0.34%     
- Complexity      487      506      +19     
============================================
  Files            46       50       +4     
  Lines          1170     1229      +59     
  Branches        103      108       +5     
============================================
+ Hits           1086     1145      +59     
  Misses           54       54              
  Partials         30       30

Flag	Coverage Δ
unittests	`93.16% <100.00%> (+0.34%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…s.java Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Simon Schrottner <[email protected]>

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Simon Schrottner <[email protected]>

@given

…with data loading Signed-off-by: Simon Schrottner <[email protected]> diff --git c/src/test/java/dev/openfeature/sdk/e2e/steps/ContextSteps.java i/src/test/java/dev/openfeature/sdk/e2e/steps/ContextSteps.java index e4cd603..121b567 100644 --- c/src/test/java/dev/openfeature/sdk/e2e/steps/ContextSteps.java +++ i/src/test/java/dev/openfeature/sdk/e2e/steps/ContextSteps.java @@ -114,9 +114,9 @@ public class ContextSteps { public void a_context_containing_a_key_with_type_and_with_value(String key, String type, String value) throws ClassNotFoundException, InstantiationException { Map<String, Value> map = state.context.asMap(); - Map<String, Value> map = state.context.asMap(); map.put(key, new Value(Utils.convert(value, type))); state.context = new MutableContext(state.context.getTargetingKey(), map); + } @given("a context containing a targeting key with value {string}") public void a_context_containing_a_targeting_key_with_value(String string) { diff --git c/src/test/java/dev/openfeature/sdk/testutils/jackson/CelContextEvaluator.java i/src/test/java/dev/openfeature/sdk/testutils/jackson/CelContextEvaluator.java index 20f3f5f..138c23f 100644 --- c/src/test/java/dev/openfeature/sdk/testutils/jackson/CelContextEvaluator.java +++ i/src/test/java/dev/openfeature/sdk/testutils/jackson/CelContextEvaluator.java @@ -46,7 +46,6 @@ public class CelContextEvaluator<T> implements ContextEvaluator<T> { // Evaluate with context, overriding defaults. objectMap.putAll(evaluationContext.asObjectMap()); } - } Object result = program.eval(objectMap); diff --git c/src/test/java/dev/openfeature/sdk/testutils/jackson/VariantsMapDeserializer.java i/src/test/java/dev/openfeature/sdk/testutils/jackson/VariantsMapDeserializer.java index 339d8c8..9a63f58 100644 --- c/src/test/java/dev/openfeature/sdk/testutils/jackson/VariantsMapDeserializer.java +++ i/src/test/java/dev/openfeature/sdk/testutils/jackson/VariantsMapDeserializer.java @@ -50,6 +50,7 @@ public class VariantsMapDeserializer extends JsonDeserializer<Map<String, Object } private Object convertJsonNodeToValue(JsonNode node) throws InstantiationException { + if (node.isNull()) { return null; } else if (node.isBoolean()) { @@ -60,25 +61,8 @@ public class VariantsMapDeserializer extends JsonDeserializer<Map<String, Object return node.asDouble(); } else if (node.isTextual()) { return node.asText(); - } else if (node.isArray()) { - List<Object> list = new ArrayList<>(); - for (JsonNode item : node) { - list.add(convertJsonNodeToValue(item)); - } - return list; } else if (node.isObject()) { - Map<String, Value> map = new HashMap<>(); - Iterator<Map.Entry<String, JsonNode>> fields = node.fields(); - while (fields.hasNext()) { - Map.Entry<String, JsonNode> field = fields.next(); - Object o = convertJsonNodeToValue(field.getValue()); - if (o instanceof Value) { - map.put(field.getKey(), (Value) o); - } else { - map.put(field.getKey(), new Value(o)); - } - } - return new Value(new MutableStructure(map)); + return Value.objectToValue(node); } throw new IllegalArgumentException("Unsupported JSON node type: " + node.getNodeType()); Signed-off-by: Simon Schrottner <[email protected]> diff --git c/src/test/java/dev/openfeature/sdk/e2e/Utils.java i/src/test/java/dev/openfeature/sdk/e2e/Utils.java index 1500d99..565968c 100644 --- c/src/test/java/dev/openfeature/sdk/e2e/Utils.java +++ i/src/test/java/dev/openfeature/sdk/e2e/Utils.java @@ -7,6 +7,8 @@ import java.util.Objects; public final class Utils { + public static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); + private Utils() {} public static Object convert(String value, String type) { @@ -27,7 +29,7 @@ public final class Utils { return Long.parseLong(value); case "object": try { - return Value.objectToValue(new ObjectMapper().readValue(value, Object.class)); + return Value.objectToValue(OBJECT_MAPPER.readValue(value, Object.class)); } catch (JsonProcessingException e) { throw new RuntimeException(e); } diff --git c/src/test/java/dev/openfeature/sdk/testutils/TestFlagsUtils.java i/src/test/java/dev/openfeature/sdk/testutils/TestFlagsUtils.java index 78a2fc5..13fe32f 100644 --- c/src/test/java/dev/openfeature/sdk/testutils/TestFlagsUtils.java +++ i/src/test/java/dev/openfeature/sdk/testutils/TestFlagsUtils.java @@ -17,6 +17,8 @@ import java.util.Map; import lombok.experimental.UtilityClass; import lombok.extern.slf4j.Slf4j; +import static dev.openfeature.sdk.e2e.Utils.OBJECT_MAPPER; + /** * Test flags utils. */ @@ -41,7 +43,8 @@ public class TestFlagsUtils { */ public static synchronized Map<String, Flag<?>> buildFlags() { if (flags == null) { - ObjectMapper objectMapper = new ObjectMapper(); + if (flags == null) { + ObjectMapper objectMapper = OBJECT_MAPPER; objectMapper.configure(StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION.mappedFeature(), true); objectMapper.addMixIn(Flag.class, InMemoryFlagMixin.class); objectMapper.addMixIn(Flag.FlagBuilder.class, InMemoryFlagMixin.FlagBuilderMixin.class); Signed-off-by: Simon Schrottner <[email protected]>

src/test/java/dev/openfeature/sdk/e2e/Utils.java

Signed-off-by: Simon Schrottner <[email protected]>

src/test/java/dev/openfeature/sdk/testutils/jackson/VariantsMapDeserializer.java

src/test/java/dev/openfeature/sdk/testutils/jackson/CelContextEvaluator.java

Signed-off-by: Simon Schrottner <[email protected]>

sonarqubecloud · 2025-09-15T13:25:02Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

toddbaert

🙏

aepfli requested review from a team as code owners August 25, 2025 06:26

gemini-code-assist bot reviewed Aug 25, 2025

View reviewed changes

aepfli force-pushed the feat/improve-inmemmory-update-gherkin-tests branch from 97a6b30 to 82a7491 Compare August 25, 2025 07:31

test: improve gherkin test suite only relying on the newest version, …

4b3c267

…with data loading Signed-off-by: Simon Schrottner <[email protected]>

aepfli force-pushed the feat/improve-inmemmory-update-gherkin-tests branch from 82a7491 to 4b3c267 Compare August 25, 2025 07:32

gemini-code-assist bot reviewed Aug 25, 2025

View reviewed changes

aepfli and others added 2 commits August 25, 2025 09:35

Update src/test/java/dev/openfeature/sdk/e2e/steps/ContextSteps.java

c7355a9

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Simon Schrottner <[email protected]>

Update src/test/java/dev/openfeature/sdk/testutils/jackson/CelContext…

04e3230

…Evaluator.java Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Simon Schrottner <[email protected]>

aepfli force-pushed the feat/improve-inmemmory-update-gherkin-tests branch 2 times, most recently from 3225636 to 770ebac Compare August 25, 2025 11:17

aepfli force-pushed the feat/improve-inmemmory-update-gherkin-tests branch from 770ebac to c57e3da Compare August 25, 2025 11:25

gemini-code-assist bot reviewed Aug 25, 2025

View reviewed changes

aepfli and others added 2 commits August 25, 2025 13:34

Update src/test/java/dev/openfeature/sdk/e2e/steps/FlagStepDefinition…

71951f8

…s.java Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Simon Schrottner <[email protected]>

Update src/test/java/dev/openfeature/sdk/testutils/TestFlagsUtils.java

cf34c77

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Simon Schrottner <[email protected]>

aepfli force-pushed the feat/improve-inmemmory-update-gherkin-tests branch 2 times, most recently from bcb53a0 to 131a24d Compare August 25, 2025 12:00

aepfli force-pushed the feat/improve-inmemmory-update-gherkin-tests branch from 131a24d to f3512f9 Compare August 25, 2025 12:01

chrfwow reviewed Aug 25, 2025

View reviewed changes

src/test/java/dev/openfeature/sdk/e2e/Utils.java Show resolved Hide resolved

fixup: add provider-status tests

e097032

Signed-off-by: Simon Schrottner <[email protected]>

aepfli force-pushed the feat/improve-inmemmory-update-gherkin-tests branch from d145f75 to e097032 Compare August 25, 2025 14:35

chrfwow reviewed Sep 2, 2025

View reviewed changes

src/test/java/dev/openfeature/sdk/testutils/jackson/VariantsMapDeserializer.java Show resolved Hide resolved

src/test/java/dev/openfeature/sdk/testutils/jackson/CelContextEvaluator.java Show resolved Hide resolved

fixup: update spec

017611b

Signed-off-by: Simon Schrottner <[email protected]>

aepfli force-pushed the feat/improve-inmemmory-update-gherkin-tests branch from 6a4535c to 3788ac4 Compare September 10, 2025 18:28

fixup: add missing steps and adapt to new spec steps

4a3faf1

Signed-off-by: Simon Schrottner <[email protected]>

aepfli force-pushed the feat/improve-inmemmory-update-gherkin-tests branch from 3788ac4 to 4a3faf1 Compare September 10, 2025 18:40

Merge branch 'main' into feat/improve-inmemmory-update-gherkin-tests

ec58624

chrfwow approved these changes Sep 11, 2025

View reviewed changes

aepfli added 2 commits September 15, 2025 11:39

Merge branch 'main' into feat/improve-inmemmory-update-gherkin-tests

1a5ffa8

fix: race condition?

af30a35

Signed-off-by: Simon Schrottner <[email protected]>

toddbaert self-requested a review September 16, 2025 13:45

toddbaert approved these changes Sep 16, 2025

View reviewed changes

toddbaert merged commit e89fab2 into main Sep 16, 2025
13 checks passed

toddbaert deleted the feat/improve-inmemmory-update-gherkin-tests branch September 16, 2025 13:45

test: improve gherkin test suite only relying on the newest version, with data loading #1578

test: improve gherkin test suite only relying on the newest version, with data loading #1578

Uh oh!

Conversation

aepfli commented Aug 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aepfli commented Aug 25, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aepfli commented Aug 25, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Aug 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud bot commented Sep 15, 2025

Quality Gate passed

Uh oh!

toddbaert left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

aepfli commented Aug 25, 2025 •

edited

Loading

codecov bot commented Aug 25, 2025 •

edited

Loading