-
Notifications
You must be signed in to change notification settings - Fork 6
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
Ensure percentage split evaluations are consistent in Core API and Local Evaluation #171
Conversation
Minimum allowed coverage is Generated by 🐒 cobertura-action against 39bb81a |
@@ -271,3 +271,63 @@ def test_build_environment_api_key_model(): | |||
|
|||
# Then | |||
assert environment_key_model.key == environment_key_dict["key"] | |||
|
|||
|
|||
def test_build_environment_model_with_deprecated_field(): |
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.
Sorry for being a pedant, but the more -> None
annotations are added now, the less we'll suffer when enforcing strict typing later
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.
Good catch, sorry! Will add now!
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.
(and never apologise for pedantry!)
# When/ Then | ||
assert identity.get_hash_key(use_mv_v2_evaluations) == identity.composite_key | ||
|
||
def test_get_hash_key_with_use_identity_composite_key_for_hashing_enabled(identity): |
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.
def test_get_hash_key_with_use_identity_composite_key_for_hashing_enabled(identity): | |
def test_get_hash_key_with_use_identity_composite_key_for_hashing_enabled( | |
identity: IdentityModel, | |
) -> None: |
|
||
# When/ Then | ||
assert identity.get_hash_key(use_mv_v2_evaluations) == identity.identifier | ||
def test_get_hash_key_with_use_identity_composite_key_for_hashing_disabled(identity): |
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.
def test_get_hash_key_with_use_identity_composite_key_for_hashing_disabled(identity): | |
def test_get_hash_key_with_use_identity_composite_key_for_hashing_disabled( | |
identity: IdentityModel, | |
) -> None: |
Replaces
use_mv_v2_evaluation
withuse_identity_composite_key_for_hashing
to ensure that percentage split evaluations are also consistent between Core, Edge and Local Evaluation.