Browse Source

fix(test): Preserve default feature flag values when patching in tests (#28686)

Change the Feature object, which is used to patch feature flag values in
unit tests, so that it only affects the named flags and preserves
default behavior for the remaining flags. Previously, patching with it
would cause all flags to default to false even if they would default to
true ordinarily.

Change affected tests to declare certain flags explicitly as false,
where the test would fail otherwise.
Ryan Skonnord 3 years ago
parent
commit
15309ad706

+ 22 - 1
src/sentry/testutils/helpers/features.py

@@ -1,10 +1,15 @@
 __all__ = ["Feature", "with_feature"]
 
+import logging
 from collections.abc import Mapping
 from contextlib import contextmanager
 
+import sentry.features
+from sentry.features.exceptions import FeatureNotRegistered
 from sentry.utils.compat.mock import patch
 
+logger = logging.getLogger(__name__)
+
 
 @contextmanager
 def Feature(names):
@@ -34,8 +39,24 @@ def Feature(names):
     elif not isinstance(names, Mapping):
         names = {k: True for k in names}
 
+    default_features = sentry.features.has
+
+    def features_override(name, *args, **kwargs):
+        if name in names:
+            return names[name]
+        else:
+            try:
+                default_value = default_features(name, *args, **kwargs)
+            except FeatureNotRegistered:
+                logger.info("Unregistered flag defaulting to False: %s", repr(name))
+                return False
+
+            if default_value:
+                logger.info("Flag defaulting to %s: %s", default_value, repr(name))
+            return default_value
+
     with patch("sentry.features.has") as features_has:
-        features_has.side_effect = lambda x, *a, **k: names.get(x, False)
+        features_has.side_effect = features_override
         yield
 
 

+ 3 - 1
tests/acceptance/test_incidents.py

@@ -32,7 +32,9 @@ class OrganizationIncidentsListTest(AcceptanceTestCase, SnubaTestCase):
             alert_rule=alert_rule,
         )
 
-        with self.feature(FEATURE_NAME):
+        features = {feature: True for feature in FEATURE_NAME}
+        features["organizations:alert-details-redesign"] = False
+        with self.feature(features):
             self.browser.get(self.path)
             self.browser.wait_until_not(".loading-indicator")
             self.browser.wait_until_not('[data-test-id="loading-placeholder"]')

+ 13 - 7
tests/acceptance/test_performance_summary.py

@@ -10,7 +10,11 @@ from sentry.utils.samples import load_data
 
 from .page_objects.transaction_summary import TransactionSummaryPage
 
-FEATURE_NAMES = ("organizations:performance-view",)
+FEATURES = {
+    "organizations:performance-view": True,
+    "organizations:performance-tag-explorer": False,
+    "organizations:performance-tag-page": False,
+}
 
 
 def make_event(event_data):
@@ -58,7 +62,7 @@ class PerformanceSummaryTest(AcceptanceTestCase, SnubaTestCase):
             project_id=self.project.id,
         )
 
-        with self.feature(FEATURE_NAMES):
+        with self.feature(FEATURES):
             self.browser.get(self.path)
             self.page.wait_until_loaded()
             # This test is flakey in that we sometimes load this page before the event is processed
@@ -81,7 +85,7 @@ class PerformanceSummaryTest(AcceptanceTestCase, SnubaTestCase):
         )
         self.store_event(data=event, project_id=self.project.id)
 
-        with self.feature(FEATURE_NAMES):
+        with self.feature(FEATURES):
             self.browser.get(self.path)
             self.page.wait_until_loaded()
 
@@ -105,7 +109,9 @@ class PerformanceSummaryTest(AcceptanceTestCase, SnubaTestCase):
         event = make_event(event_data)
         self.store_event(data=event, project_id=self.project.id)
 
-        with self.feature(FEATURE_NAMES + ("organizations:performance-tag-page",)):
+        features = dict(FEATURES)
+        features["organizations:performance-tag-page"] = True
+        with self.feature(features):
             self.browser.get(tags_path)
             self.page.wait_until_loaded()
             self.browser.snapshot("transaction summary tags page")
@@ -127,7 +133,7 @@ class PerformanceSummaryTest(AcceptanceTestCase, SnubaTestCase):
         event = make_event(event_data)
         self.store_event(data=event, project_id=self.project.id)
 
-        with self.feature(FEATURE_NAMES):
+        with self.feature(FEATURES):
             self.browser.get(vitals_path)
             self.page.wait_until_loaded()
 
@@ -185,7 +191,7 @@ class PerformanceSummaryTest(AcceptanceTestCase, SnubaTestCase):
         event_data["measurements"]["cls"]["value"] = 3000000000
         self.store_event(data=event_data, project_id=self.project.id)
 
-        with self.feature(FEATURE_NAMES):
+        with self.feature(FEATURES):
             self.browser.get(vitals_path)
             self.page.wait_until_loaded()
 
@@ -215,7 +221,7 @@ class PerformanceSummaryTest(AcceptanceTestCase, SnubaTestCase):
             project_id=self.project.id,
         )
 
-        with self.feature(FEATURE_NAMES):
+        with self.feature(FEATURES):
             self.browser.get(self.path)
             self.page.wait_until_loaded()
             # This test is flakey in that we sometimes load this page before the event is processed

+ 24 - 4
tests/sentry/api/endpoints/test_group_integration_details.py

@@ -156,7 +156,12 @@ class GroupIntegrationDetailsTest(APITestCase):
 
         path = f"/api/0/issues/{self.group.id}/integrations/{integration.id}/?action=create"
 
-        with self.feature({"organizations:integrations-issue-basic": False}):
+        with self.feature(
+            {
+                "organizations:integrations-issue-basic": False,
+                "organizations:integrations-issue-sync": False,
+            }
+        ):
             response = self.client.get(path)
         assert response.status_code == 400
         assert response.data["detail"] == "Your organization does not have access to this feature."
@@ -205,7 +210,12 @@ class GroupIntegrationDetailsTest(APITestCase):
 
         path = f"/api/0/issues/{group.id}/integrations/{integration.id}/"
 
-        with self.feature({"organizations:integrations-issue-basic": False}):
+        with self.feature(
+            {
+                "organizations:integrations-issue-basic": False,
+                "organizations:integrations-issue-sync": False,
+            }
+        ):
             response = self.client.put(path, data={"externalIssue": "APP-123"})
         assert response.status_code == 400
         assert response.data["detail"] == "Your organization does not have access to this feature."
@@ -260,7 +270,12 @@ class GroupIntegrationDetailsTest(APITestCase):
 
         path = f"/api/0/issues/{group.id}/integrations/{integration.id}/"
 
-        with self.feature({"organizations:integrations-issue-basic": False}):
+        with self.feature(
+            {
+                "organizations:integrations-issue-basic": False,
+                "organizations:integrations-issue-sync": False,
+            }
+        ):
             response = self.client.post(path, data={})
         assert response.status_code == 400
         assert response.data["detail"] == "Your organization does not have access to this feature."
@@ -314,7 +329,12 @@ class GroupIntegrationDetailsTest(APITestCase):
 
         path = f"/api/0/issues/{group.id}/integrations/{integration.id}/?externalIssue={external_issue.id}"
 
-        with self.feature({"organizations:integrations-issue-basic": False}):
+        with self.feature(
+            {
+                "organizations:integrations-issue-basic": False,
+                "organizations:integrations-issue-sync": False,
+            }
+        ):
             response = self.client.delete(path)
         assert response.status_code == 400
         assert response.data["detail"] == "Your organization does not have access to this feature."

+ 6 - 1
tests/sentry/api/endpoints/test_group_integrations.py

@@ -82,6 +82,11 @@ class GroupIntegrationsTest(APITestCase):
 
         path = f"/api/0/issues/{group.id}/integrations/"
 
-        with self.feature({"organizations:integrations-issue-basic": False}):
+        with self.feature(
+            {
+                "organizations:integrations-issue-basic": False,
+                "organizations:integrations-issue-sync": False,
+            }
+        ):
             response = self.client.get(path)
         assert response.data == []

+ 3 - 1
tests/sentry/api/endpoints/test_organization_dashboards.py

@@ -168,7 +168,9 @@ class OrganizationDashboardsTest(OrganizationDashboardWidgetTestCase):
         assert response.status_code == 201
 
     def test_post_features_required(self):
-        with self.feature({"organizations:dashboards-basic": False}):
+        with self.feature(
+            {"organizations:dashboards-basic": False, "organizations:dashboards-edit": False}
+        ):
             response = self.do_request(
                 "post",
                 self.url,

+ 14 - 2
tests/sentry/api/endpoints/test_project_rules_configuration.py

@@ -98,7 +98,13 @@ class ProjectRuleConfigurationTest(APITestCase):
             assert JIRA_ACTION not in action_ids
 
     def test_percent_condition_flag(self):
-        with self.feature({"organizations:issue-percent-filters": False}):
+        with self.feature(
+            {
+                "projects:alert-filters": False,
+                "organizations:integrations-ticket-rules": False,
+                "organizations:issue-percent-filters": False,
+            }
+        ):
             # We should not get back the condition.
             response = self.get_valid_response(self.organization.slug, self.project.slug)
             assert len(response.data["conditions"]) == 9
@@ -108,7 +114,13 @@ class ProjectRuleConfigurationTest(APITestCase):
                     != "sentry.rules.conditions.event_frequency.EventFrequencyPercentCondition"
                 )
 
-        with self.feature({"organizations:issue-percent-filters": True}):
+        with self.feature(
+            {
+                "projects:alert-filters": False,
+                "organizations:integrations-ticket-rules": False,
+                "organizations:issue-percent-filters": True,
+            }
+        ):
             # We should get back the condition.
             response = self.get_valid_response(self.organization.slug, self.project.slug)
             assert len(response.data["conditions"]) == 10

+ 1 - 1
tests/sentry/api/endpoints/test_project_transaction_threshold_override.py

@@ -72,7 +72,7 @@ class ProjectTransactionThresholdOverrideTest(APITestCase):
         assert response.status_code == 404
 
     def test_get_returns_error_without_feature_enabled(self):
-        with self.feature({self.feature_name: False}):
+        with self.feature({self.feature_name: False, "organizations:discover-basic": False}):
             ProjectTransactionThresholdOverride.objects.create(
                 project=self.project,
                 organization=self.project.organization,

+ 23 - 21
tests/snuba/api/endpoints/test_organization_events_facets_performance.py

@@ -7,7 +7,7 @@ from sentry.testutils.helpers.datetime import before_now, iso_format
 from sentry.utils.samples import load_data
 
 
-class OrganizationEventsFacetsPerformanceEndpointTest(SnubaTestCase, APITestCase):
+class BaseOrganizationEventsFacetsPerformanceEndpointTest(SnubaTestCase, APITestCase):
     feature_list = (
         "organizations:discover-basic",
         "organizations:global-views",
@@ -24,6 +24,22 @@ class OrganizationEventsFacetsPerformanceEndpointTest(SnubaTestCase, APITestCase
         self.project = self.create_project()
         self.project2 = self.create_project()
 
+    def do_request(self, query=None, features=None):
+        query = query if query is not None else {"aggregateColumn": "transaction.duration"}
+        query["project"] = query["project"] if "project" in query else [self.project.id]
+
+        feature_dict = {feature: True for feature in self.feature_list}
+        feature_dict.update(features or {})
+        with self.feature(feature_dict):
+            return self.client.get(self.url, query, format="json")
+
+
+class OrganizationEventsFacetsPerformanceEndpointTest(
+    BaseOrganizationEventsFacetsPerformanceEndpointTest
+):
+    def setUp(self):
+        super().setUp()
+
         self._transaction_count = 0
 
         for i in range(5):
@@ -68,12 +84,6 @@ class OrganizationEventsFacetsPerformanceEndpointTest(SnubaTestCase, APITestCase
         self._transaction_count += 1
         self.store_event(data=event, project_id=project_id)
 
-    def do_request(self, query=None, feature_list=None):
-        query = query if query is not None else {"aggregateColumn": "transaction.duration"}
-        query["project"] = query["project"] if "project" in query else [self.project.id]
-        with self.feature(feature_list or self.feature_list):
-            return self.client.get(self.url, query, format="json")
-
     def test_basic_request(self):
         response = self.do_request()
         assert response.status_code == 200, response.content
@@ -185,7 +195,7 @@ class OrganizationEventsFacetsPerformanceEndpointTest(SnubaTestCase, APITestCase
             "allTagKeys": True,
         }
         # No feature access
-        response = self.do_request(request)
+        response = self.do_request(request, {"organizations:performance-tag-page": False})
         data = response.data["data"]
         assert len(data) == 1
         assert data[0]["count"] == 5
@@ -193,9 +203,7 @@ class OrganizationEventsFacetsPerformanceEndpointTest(SnubaTestCase, APITestCase
         assert data[0]["tags_value"] == "blue"
 
         # With feature access
-        response = self.do_request(
-            request, feature_list=self.feature_list + ("organizations:performance-tag-page",)
-        )
+        response = self.do_request(request, {"organizations:performance-tag-page": True})
         data = response.data["data"]
         assert len(data) == 5
         assert data[0]["count"] == 19
@@ -215,9 +223,7 @@ class OrganizationEventsFacetsPerformanceEndpointTest(SnubaTestCase, APITestCase
             "allTagKeys": True,
         }
 
-        response = self.do_request(
-            request, feature_list=self.feature_list + ("organizations:performance-tag-page",)
-        )
+        response = self.do_request(request, {"organizations:performance-tag-page": True})
 
         data = response.data["data"]
         assert len(data) == 5
@@ -237,7 +243,7 @@ class OrganizationEventsFacetsPerformanceEndpointTest(SnubaTestCase, APITestCase
             "tagKey": "color",
         }
         # No feature access
-        response = self.do_request(request)
+        response = self.do_request(request, {"organizations:performance-tag-page": False})
         data = response.data["data"]
         assert len(data) == 2
         assert data[0]["count"] == 5
@@ -245,9 +251,7 @@ class OrganizationEventsFacetsPerformanceEndpointTest(SnubaTestCase, APITestCase
         assert data[0]["tags_value"] == "blue"
 
         # With feature access
-        response = self.do_request(
-            request, feature_list=self.feature_list + ("organizations:performance-tag-page",)
-        )
+        response = self.do_request(request, {"organizations:performance-tag-page": True})
         data = response.data["data"]
         assert len(data) == 3
         assert data[0]["count"] == 14
@@ -267,9 +271,7 @@ class OrganizationEventsFacetsPerformanceEndpointTest(SnubaTestCase, APITestCase
             "query": "(color:purple)",
         }
 
-        response = self.do_request(
-            request, feature_list=self.feature_list + ("organizations:performance-tag-page",)
-        )
+        response = self.do_request(request, {"organizations:performance-tag-page": True})
         data = response.data["data"]
         assert len(data) == 1
         assert data[0]["count"] == 1

+ 26 - 62
tests/snuba/api/endpoints/test_organization_events_facets_performance_histogram.py

@@ -2,13 +2,17 @@ from datetime import timedelta
 
 from django.urls import reverse
 
-from sentry.testutils import APITestCase, SnubaTestCase
-from sentry.testutils.helpers.datetime import before_now, iso_format
+from sentry.testutils.helpers.datetime import iso_format
 from sentry.utils.cursors import Cursor
 from sentry.utils.samples import load_data
+from tests.snuba.api.endpoints.test_organization_events_facets_performance import (
+    BaseOrganizationEventsFacetsPerformanceEndpointTest,
+)
 
 
-class OrganizationEventsFacetsPerformanceHistogramEndpointTest(SnubaTestCase, APITestCase):
+class OrganizationEventsFacetsPerformanceHistogramEndpointTest(
+    BaseOrganizationEventsFacetsPerformanceEndpointTest
+):
     feature_list = (
         "organizations:discover-basic",
         "organizations:global-views",
@@ -17,13 +21,6 @@ class OrganizationEventsFacetsPerformanceHistogramEndpointTest(SnubaTestCase, AP
 
     def setUp(self):
         super().setUp()
-        self.min_ago = before_now(minutes=1).replace(microsecond=0)
-        self.two_mins_ago = before_now(minutes=2).replace(microsecond=0)
-        self.day_ago = before_now(days=1).replace(microsecond=0)
-
-        self.login_as(user=self.user)
-        self.project = self.create_project()
-        self.project2 = self.create_project()
 
         self._transaction_count = 0
 
@@ -90,12 +87,6 @@ class OrganizationEventsFacetsPerformanceHistogramEndpointTest(SnubaTestCase, AP
         self._transaction_count += 1
         self.store_event(data=event, project_id=project_id)
 
-    def do_request(self, query=None, feature_list=None):
-        query = query if query is not None else {"aggregateColumn": "transaction.duration"}
-        query["project"] = query["project"] if "project" in query else [self.project.id]
-        with self.feature(feature_list or self.feature_list):
-            return self.client.get(self.url, query, format="json")
-
     def test_multiple_projects_not_allowed(self):
         response = self.do_request(
             {
@@ -123,10 +114,11 @@ class OrganizationEventsFacetsPerformanceHistogramEndpointTest(SnubaTestCase, AP
         }
         error_response = self.do_request(
             request,
-            feature_list=(
-                "organizations:discover-basic",
-                "organizations:global-views",
-            ),
+            {
+                "organizations:discover-basic": True,
+                "organizations:global-views": True,
+                "organizations:performance-tag-page": False,
+            },
         )
         assert error_response.status_code == 404
 
@@ -140,9 +132,7 @@ class OrganizationEventsFacetsPerformanceHistogramEndpointTest(SnubaTestCase, AP
             "per_page": 5,
         }
         # With feature access, no tag key
-        error_response = self.do_request(
-            request, feature_list=self.feature_list + ("organizations:performance-tag-page",)
-        )
+        error_response = self.do_request(request, {"organizations:performance-tag-page": True})
 
         assert error_response.status_code == 400, error_response.content
         assert error_response.data == {
@@ -160,18 +150,14 @@ class OrganizationEventsFacetsPerformanceHistogramEndpointTest(SnubaTestCase, AP
             "query": "(color:red or color:blue)",
         }
         # With feature access, no tag key
-        error_response = self.do_request(
-            request, feature_list=self.feature_list + ("organizations:performance-tag-page",)
-        )
+        error_response = self.do_request(request, {"organizations:performance-tag-page": True})
 
         assert error_response.status_code == 400, error_response.content
         assert error_response.data == {"detail": "'tagKey' must be provided when using histograms."}
 
         # With feature access and tag key
         request["tagKey"] = "color"
-        data_response = self.do_request(
-            request, feature_list=self.feature_list + ("organizations:performance-tag-page",)
-        )
+        data_response = self.do_request(request, {"organizations:performance-tag-page": True})
 
         histogram_data = data_response.data["histogram"]["data"]
         assert len(histogram_data) == 2
@@ -202,9 +188,7 @@ class OrganizationEventsFacetsPerformanceHistogramEndpointTest(SnubaTestCase, AP
             "query": "(color:teal or color:oak)",
         }
 
-        data_response = self.do_request(
-            request, feature_list=self.feature_list + ("organizations:performance-tag-page",)
-        )
+        data_response = self.do_request(request, {"organizations:performance-tag-page": True})
 
         histogram_data = data_response.data["histogram"]["data"]
         assert histogram_data == []
@@ -224,9 +208,7 @@ class OrganizationEventsFacetsPerformanceHistogramEndpointTest(SnubaTestCase, AP
             "query": "(color:red or color:blue or color:green)",
         }
 
-        data_response = self.do_request(
-            request, feature_list=self.feature_list + ("organizations:performance-tag-page",)
-        )
+        data_response = self.do_request(request, {"organizations:performance-tag-page": True})
 
         histogram_data = data_response.data["histogram"]["data"]
         assert len(histogram_data) == 1
@@ -236,9 +218,7 @@ class OrganizationEventsFacetsPerformanceHistogramEndpointTest(SnubaTestCase, AP
         assert histogram_data[0]["tags_key"] == "color"
 
         request["per_page"] = 3
-        data_response = self.do_request(
-            request, feature_list=self.feature_list + ("organizations:performance-tag-page",)
-        )
+        data_response = self.do_request(request, {"organizations:performance-tag-page": True})
 
         histogram_data = data_response.data["histogram"]["data"]
         assert len(histogram_data) == 3
@@ -270,17 +250,13 @@ class OrganizationEventsFacetsPerformanceHistogramEndpointTest(SnubaTestCase, AP
             "query": "(color:red or color:blue or color:green or color:orange)",
         }
 
-        data_response = self.do_request(
-            request, feature_list=self.feature_list + ("organizations:performance-tag-page",)
-        )
+        data_response = self.do_request(request, {"organizations:performance-tag-page": True})
 
         assert data_response.data["tags"]["data"][2]["tags_value"] == "green"
 
         request["aggregateColumn"] = "measurements.lcp"
 
-        data_response = self.do_request(
-            request, feature_list=self.feature_list + ("organizations:performance-tag-page",)
-        )
+        data_response = self.do_request(request, {"organizations:performance-tag-page": True})
 
         tags_data = data_response.data["tags"]["data"]
         assert len(tags_data) == 3
@@ -317,9 +293,7 @@ class OrganizationEventsFacetsPerformanceHistogramEndpointTest(SnubaTestCase, AP
             "query": "(user.id:555)",
         }
 
-        data_response = self.do_request(
-            request, feature_list=self.feature_list + ("organizations:performance-tag-page",)
-        )
+        data_response = self.do_request(request, {"organizations:performance-tag-page": True})
 
         histogram_data = data_response.data["histogram"]["data"]
         assert histogram_data[0]["count"] == 1
@@ -339,18 +313,14 @@ class OrganizationEventsFacetsPerformanceHistogramEndpointTest(SnubaTestCase, AP
             "tagKey": "color",
         }
 
-        data_response = self.do_request(
-            request, feature_list=self.feature_list + ("organizations:performance-tag-page",)
-        )
+        data_response = self.do_request(request, {"organizations:performance-tag-page": True})
 
         tag_data = data_response.data["tags"]["data"]
         assert len(tag_data) == 3
 
         request["cursor"] = Cursor(0, 3)
 
-        data_response = self.do_request(
-            request, feature_list=self.feature_list + ("organizations:performance-tag-page",)
-        )
+        data_response = self.do_request(request, {"organizations:performance-tag-page": True})
 
         tag_data = data_response.data["tags"]["data"]
         assert len(tag_data) == 1
@@ -365,9 +335,7 @@ class OrganizationEventsFacetsPerformanceHistogramEndpointTest(SnubaTestCase, AP
             "tagKey": "color",
         }
 
-        data_response = self.do_request(
-            request, feature_list=self.feature_list + ("organizations:performance-tag-page",)
-        )
+        data_response = self.do_request(request, {"organizations:performance-tag-page": True})
 
         tag_data = data_response.data["tags"]["data"]
         assert len(tag_data) == 1
@@ -376,9 +344,7 @@ class OrganizationEventsFacetsPerformanceHistogramEndpointTest(SnubaTestCase, AP
 
         request["sort"] = "-aggregate"
 
-        data_response = self.do_request(
-            request, feature_list=self.feature_list + ("organizations:performance-tag-page",)
-        )
+        data_response = self.do_request(request, {"organizations:performance-tag-page": True})
 
         tag_data = data_response.data["tags"]["data"]
         assert len(tag_data) == 1
@@ -398,9 +364,7 @@ class OrganizationEventsFacetsPerformanceHistogramEndpointTest(SnubaTestCase, AP
             "tagKey": "fruit",
         }
 
-        data_response = self.do_request(
-            request, feature_list=self.feature_list + ("organizations:performance-tag-page",)
-        )
+        data_response = self.do_request(request, {"organizations:performance-tag-page": True})
 
         histogram_data = data_response.data["histogram"]["data"]
         assert len(histogram_data) == 20

Some files were not shown because too many files changed in this diff