diff --git a/flagsmith/flagsmith.py b/flagsmith/flagsmith.py index fc8a29b..c5860d8 100644 --- a/flagsmith/flagsmith.py +++ b/flagsmith/flagsmith.py @@ -362,11 +362,11 @@ def get_experiment_flag( if not self._event_processor: raise ValueError("Events must be enabled to use experiment flags.") flag = self.get_identity_flags(identifier, traits).get_flag(feature_name) - if not flag.is_default: + if isinstance(flag, Flag): self.track_exposure_event( feature_name=feature_name, identifier=identifier, - value=flag.value, + value=flag.variant if flag.variant is not None else flag.value, traits=traits, ) return flag diff --git a/tests/conftest.py b/tests/conftest.py index a71c3de..4631958 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -23,6 +23,32 @@ DATA_DIR = os.path.join(os.path.dirname(__file__), "data") +@pytest.fixture(autouse=True) +def stop_flagsmith_background_threads( + monkeypatch: pytest.MonkeyPatch, +) -> Generator[None, None, None]: + # Flagsmith starts polling, streaming and event-processor background + # threads. Track every instance created during a test and stop its threads + # on teardown so they don't leak across the suite (mirroring __del__, but + # deterministically rather than at GC). + instances: typing.List[Flagsmith] = [] + original_init = Flagsmith.__init__ + + def tracking_init(self: Flagsmith, *args: typing.Any, **kwargs: typing.Any) -> None: + instances.append(self) + original_init(self, *args, **kwargs) + + monkeypatch.setattr(Flagsmith, "__init__", tracking_init) + yield + for flagsmith in instances: + if getattr(flagsmith, "environment_data_polling_manager_thread", None): + flagsmith.environment_data_polling_manager_thread.stop() + if getattr(flagsmith, "event_stream_thread", None): + flagsmith.event_stream_thread.stop() + if flagsmith._event_processor: + flagsmith._event_processor.stop() + + @pytest.fixture() def analytics_processor() -> AnalyticsProcessor: return AnalyticsProcessor( diff --git a/tests/test_flagsmith.py b/tests/test_flagsmith.py index bdd2754..6703e14 100644 --- a/tests/test_flagsmith.py +++ b/tests/test_flagsmith.py @@ -959,12 +959,8 @@ def test_track_event_raises_without_config(api_key: str) -> None: def test_track_event_rejects_reserved_prefix(api_key: str) -> None: flagsmith = Flagsmith(environment_key=api_key, enable_events=True) - try: - with pytest.raises(ValueError, match='reserved "\\$" prefix'): - flagsmith.track_event("$made_up") - finally: - if flagsmith._event_processor: - flagsmith._event_processor.stop() + with pytest.raises(ValueError, match='reserved "\\$" prefix'): + flagsmith.track_event("$made_up") def test_event_processor_config_without_enable_events_raises(api_key: str) -> None: @@ -978,15 +974,11 @@ def test_event_processor_config_without_enable_events_raises(api_key: str) -> No def test_enable_events_without_config_uses_default(api_key: str) -> None: flagsmith = Flagsmith(environment_key=api_key, enable_events=True) - try: - assert flagsmith._event_processor is not None - assert ( - flagsmith._event_processor._batch_endpoint - == "https://events.api.flagsmith.com/v1/events" - ) - finally: - if flagsmith._event_processor: - flagsmith._event_processor.stop() + assert flagsmith._event_processor is not None + assert ( + flagsmith._event_processor._batch_endpoint + == "https://events.api.flagsmith.com/v1/events" + ) def test_track_event_delegates_to_event_processor( @@ -1116,3 +1108,73 @@ def default_flag_handler(feature_name: str) -> DefaultFlag: assert result.is_default is True assert result.value == "default-variant" mock_track.assert_not_called() + + +def test_get_experiment_flag_uses_variant_as_exposure_value( + mocker: MockerFixture, api_key: str +) -> None: + # Given - a resolved flag carrying a variant + config = EventProcessorConfig(events_api_url="http://test/") + flagsmith = Flagsmith( + environment_key=api_key, enable_events=True, event_processor_config=config + ) + flag = Flag( + enabled=True, + value="blue", + feature_name="checkout_v2", + feature_id=1, + variant="control", + ) + mocker.patch.object( + flagsmith, + "get_identity_flags", + return_value=Flags(flags={"checkout_v2": flag}), + ) + mock_track = mocker.patch.object(flagsmith._event_processor, "track_exposure_event") + + # When + flagsmith.get_experiment_flag(feature_name="checkout_v2", identifier="user1") + + # Then - the exposure value is the variant, not the flag value + mock_track.assert_called_once_with( + feature_name="checkout_v2", + identifier="user1", + value="control", + traits=None, + metadata=None, + ) + + +def test_get_experiment_flag_falls_back_to_value_without_variant( + mocker: MockerFixture, api_key: str +) -> None: + # Given - a resolved flag with no variant + config = EventProcessorConfig(events_api_url="http://test/") + flagsmith = Flagsmith( + environment_key=api_key, enable_events=True, event_processor_config=config + ) + flag = Flag( + enabled=True, + value="blue", + feature_name="checkout_v2", + feature_id=1, + variant=None, + ) + mocker.patch.object( + flagsmith, + "get_identity_flags", + return_value=Flags(flags={"checkout_v2": flag}), + ) + mock_track = mocker.patch.object(flagsmith._event_processor, "track_exposure_event") + + # When + flagsmith.get_experiment_flag(feature_name="checkout_v2", identifier="user1") + + # Then - the exposure value falls back to the flag value + mock_track.assert_called_once_with( + feature_name="checkout_v2", + identifier="user1", + value="blue", + traits=None, + metadata=None, + )