-
Notifications
You must be signed in to change notification settings - Fork 42
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
feat: Add new test suite for context merging #293
Conversation
Signed-off-by: christian.lutnik <[email protected]>
There was a problem hiding this 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]>
Signed-off-by: christian.lutnik <[email protected]>
There was a problem hiding this 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.
@aepfli when I annotate a test with both |
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]>
Signed-off-by: christian.lutnik <[email protected]>
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