Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add new test suite for context merging #293

Conversation

chrfwow
Copy link
Contributor

@chrfwow chrfwow commented Feb 6, 2025

This PR

Adds gherkin tests to verify the merging order of contexts: https://openfeature.dev/specification/sections/evaluation-context/#requirement-323

See open-feature/flagd-testbed#13

@chrfwow chrfwow requested a review from toddbaert as a code owner February 6, 2025 13:25
Copy link
Member

@aepfli aepfli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, i think those gherkin tests will make our live easier in the future. But we have to be a little bit more careful how we enable certain tests with tagging. not all sdks support the whole feature set, and we want them to use this files and limit the usage. We should think about how to properly tag certain tests, or move them in separate scenarios. eg. transaction context merging

Signed-off-by: christian.lutnik <[email protected]>
@chrfwow chrfwow requested a review from aepfli February 26, 2025 07:19
Signed-off-by: christian.lutnik <[email protected]>
Copy link
Member

@aepfli aepfli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, we might want to have also one test with @Hooks @Transaction testing both together - i know it is another duplication, and to be honest, i do not have a hard feeling for or against them. Thank you for improving our gherkin tests.

@chrfwow
Copy link
Contributor Author

chrfwow commented Mar 3, 2025

@aepfli when I annotate a test with both @Hooks and @Transaction, will it be executed when both tags are enabled, or when one of the tags is enabled?

@aepfli
Copy link
Member

aepfli commented Mar 3, 2025

@aepfli when I annotate a test with both @Hooks and @Transaction, will it be executed when both tags are enabled, or when one of the tags is enabled?

you can say eg. "Transaction and not Hooks" or simply "not hooks" if a feature is disabled, so you can be flexible with the selection of tags

Signed-off-by: christian.lutnik <[email protected]>
@chrfwow chrfwow requested review from aepfli and toddbaert March 3, 2025 13:44
Signed-off-by: christian.lutnik <[email protected]>
@toddbaert toddbaert merged commit 25c57ee into open-feature:main Mar 6, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants