Conversation
Still need caseinsensitive on rule
tdumitrescu
left a comment
tdumitrescu
left a comment
There was a problem hiding this comment.
Thanks for the thorough test cases. Mostly just some nitpicks on my end!
test/flags/local_flags.js
Outdated
| }); | ||
|
|
||
| const result = provider.getVariant(FLAG_KEY, FALLBACK, context); | ||
| expect(result.variant_value).not.toBe(FALLBACK_NAME); |
There was a problem hiding this comment.
Can we look for a specific value here rather than just "not the fallback"? Since that could pass the test for weird failure cases like if variant_value is unexpectedly null
There was a problem hiding this comment.
Good thinking, just refactored to use a helper assertion that asserts both. Note that because there's a 50/50 split on the variant, I just check that the result is one of the two. The hashing logic is tested elsewhere and is an orthogonal concern to these test cases.
Co-authored-by: teddddd <ted@mixpanel.com>
…ode into runtime-rules-engine
joshua-koehler
left a comment
joshua-koehler
left a comment
There was a problem hiding this comment.
Thanks for the thorough review, ready for a second look @tdumitrescu
test/flags/local_flags.js
Outdated
| }); | ||
|
|
||
| const result = provider.getVariant(FLAG_KEY, FALLBACK, context); | ||
| expect(result.variant_value).not.toBe(FALLBACK_NAME); |
There was a problem hiding this comment.
Good thinking, just refactored to use a helper assertion that asserts both. Note that because there's a 50/50 split on the variant, I just check that the result is one of the two. The hashing logic is tested elsewhere and is an orthogonal concern to these test cases.
tdumitrescu
left a comment
tdumitrescu
left a comment
There was a problem hiding this comment.
LGTM, just a few little cleanup comments. Thanks!
Co-authored-by: teddddd <ted@mixpanel.com>
…ode into runtime-rules-engine
4 participants
Background
Local evaluation already supports evaluating the legacy runtime rule, which was simple exact key-value matching.
This pr
Support complex runtime rules in local-evaluation using jsonLogic.
QA'd local and remote eval ✅
https://github.com/mixpanel/sdk-integration-testing/commit/fe6d0968da0d4f30ce482abf46906d82bf7cc373
Example of what they look like in the UI