Skip to content

Conversation

@joshua-koehler
Copy link
Contributor

@joshua-koehler joshua-koehler commented Nov 17, 2025

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

image

Copy link

@msiebert msiebert left a comment

Choose a reason for hiding this comment

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

Copy link
Member

@tdumitrescu tdumitrescu left a comment

Choose a reason for hiding this comment

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

Thanks for the thorough test cases. Mostly just some nitpicks on my end!

});

const result = provider.getVariant(FLAG_KEY, FALLBACK, context);
expect(result.variant_value).not.toBe(FALLBACK_NAME);
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@joshua-koehler joshua-koehler Jan 8, 2026

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@joshua-koehler joshua-koehler left a comment

Choose a reason for hiding this comment

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

Thanks for the thorough review, ready for a second look @tdumitrescu

});

const result = provider.getVariant(FLAG_KEY, FALLBACK, context);
expect(result.variant_value).not.toBe(FALLBACK_NAME);
Copy link
Contributor Author

@joshua-koehler joshua-koehler Jan 8, 2026

Choose a reason for hiding this comment

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

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.

Copy link
Member

@tdumitrescu tdumitrescu left a comment

Choose a reason for hiding this comment

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

LGTM, just a few little cleanup comments. Thanks!

@joshua-koehler joshua-koehler merged commit b3b2f89 into master Jan 12, 2026
6 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.

5 participants