Skip to content
Draft
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion flagsmith/flagsmith.py
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,20 @@ def _get_environment_flags_from_document(self) -> Flags:
if self._evaluation_context is None:
raise TypeError("No environment present")

evaluation_result = engine.get_evaluation_result(self._evaluation_context)
# Omit segments from evaluation context for environment flags
# as they are only relevant for identity-specific evaluations
context_without_segments = typing.cast(
SDKEvaluationContext,
{
key: value
for key, value in self._evaluation_context.items()
if key != "segments"
},
)
Copy link
Member

Choose a reason for hiding this comment

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

This helps to avoid both the cast and a bytecode loop:

Suggested change
context_without_segments = typing.cast(
SDKEvaluationContext,
{
key: value
for key, value in self._evaluation_context.items()
if key != "segments"
},
)
context_without_segments = self._evaluation_context.copy()
context_without_segments.pop("segments", None)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 10242fb


evaluation_result = engine.get_evaluation_result(
context=context_without_segments,
)

return Flags.from_evaluation_result(
evaluation_result=evaluation_result,
Expand Down
75 changes: 75 additions & 0 deletions tests/test_flagsmith.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,41 @@ def test_get_environment_flags_uses_local_environment_when_available(
assert all_flags[0].value == "some-value"


def test_get_environment_flags_omits_segments_from_evaluation_context(
mocker: MockerFixture,
flagsmith: Flagsmith,
evaluation_context: SDKEvaluationContext,
) -> None:
# Given
flagsmith._evaluation_context = evaluation_context
flagsmith.enable_local_evaluation = True
mock_engine = mocker.patch("flagsmith.flagsmith.engine")

expected_evaluation_result = {
"flags": {
"some_feature": {
"name": "some_feature",
"enabled": True,
"value": "some-feature-state-value",
"metadata": {"id": 1},
}
},
"segments": [],
}

mock_engine.get_evaluation_result.return_value = expected_evaluation_result

# When
flagsmith.get_environment_flags()

# Then
mock_engine.get_evaluation_result.assert_called_once()
call_args = mock_engine.get_evaluation_result.call_args
context = call_args[1]["context"] # Keyword argument 'context'
Copy link
Member

Choose a reason for hiding this comment

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

Use assert_called_once_with method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 10242fb

# Segments should not be present in the context passed to the engine
assert "segments" not in context


@responses.activate()
def test_get_identity_flags_calls_api_when_no_local_environment_no_traits(
flagsmith: Flagsmith, identities_json: str
Expand Down Expand Up @@ -191,6 +226,46 @@ def test_get_identity_flags_uses_local_environment_when_available(
assert identity_flags[0].value == "some-feature-state-value"


def test_get_identity_flags_includes_segments_in_evaluation_context(
mocker: MockerFixture,
flagsmith: Flagsmith,
evaluation_context: SDKEvaluationContext,
) -> None:
# Given
flagsmith._evaluation_context = evaluation_context
flagsmith.enable_local_evaluation = True
Copy link
Member

Choose a reason for hiding this comment

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

Use local_eval_flagsmith fixture instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 10242fb

mock_engine = mocker.patch("flagsmith.flagsmith.engine")
Copy link
Member

Choose a reason for hiding this comment

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

Mock specific function and use autospec:

Suggested change
mock_engine = mocker.patch("flagsmith.flagsmith.engine")
mock_get_evaluation_result = mocker.patch(
"flagsmith.flagsmith.engine.get_evaluation_result",
autospec=True,
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 10242fb


expected_evaluation_result = {
"flags": {
"some_feature": {
"name": "some_feature",
"enabled": True,
"value": "some-feature-state-value",
"metadata": {"id": 1},
}
},
"segments": [],
}

identifier = "identifier"
traits = {"some_trait": "some_value"}

mock_engine.get_evaluation_result.return_value = expected_evaluation_result

# When
flagsmith.get_identity_flags(identifier, traits)

# Then
mock_engine.get_evaluation_result.assert_called_once()
call_args = mock_engine.get_evaluation_result.call_args
context = call_args[1]["context"]
Copy link
Member

Choose a reason for hiding this comment

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

Use assert_called_once_with method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 10242fb

# Segments should be present in the context passed to the engine for identity flags
assert "segments" in context
# Verify segments from evaluation_context are preserved
assert context["segments"] == evaluation_context["segments"]


@responses.activate()
def test_get_identity_flags__transient_identity__calls_expected(
flagsmith: Flagsmith,
Expand Down