ferruzzi commented on code in PR #36250:
URL: https://github.com/apache/airflow/pull/36250#discussion_r1450974993


##########
tests/core/test_stats.py:
##########
@@ -265,40 +271,128 @@ def 
test_does_not_send_stats_using_statsd_when_statsd_and_dogstatsd_both_on(self
         importlib.reload(airflow.stats)
 
 
-class TestStatsWithAllowList:
-    def setup_method(self):
-        self.statsd_client = Mock(spec=statsd.StatsClient)
-        self.stats = SafeStatsdLogger(self.statsd_client, 
AllowListValidator("stats_one, stats_two"))
+class TestStatsAllowAndBlockLists:
+    @pytest.mark.parametrize(
+        "validator, stat_name, expect_incr",
+        [
+            (PatternAllowListValidator, "stats_one", True),
+            (PatternAllowListValidator, "stats_two.bla", True),
+            (PatternAllowListValidator, "stats_three.foo", True),
+            (PatternAllowListValidator, "stats_foo_three", True),
+            (PatternAllowListValidator, "stats_three", False),
+            (AllowListValidator, "stats_one", True),
+            (AllowListValidator, "stats_two.bla", True),
+            (AllowListValidator, "stats_three.foo", False),
+            (AllowListValidator, "stats_foo_three", False),
+            (AllowListValidator, "stats_three", False),
+            (PatternBlockListValidator, "stats_one", False),
+            (PatternBlockListValidator, "stats_two.bla", False),
+            (PatternBlockListValidator, "stats_three.foo", False),
+            (PatternBlockListValidator, "stats_foo_three", False),
+            (PatternBlockListValidator, "stats_foo", False),
+            (PatternBlockListValidator, "stats_three", True),
+            (BlockListValidator, "stats_one", False),
+            (BlockListValidator, "stats_two.bla", False),
+            (BlockListValidator, "stats_three.foo", True),
+            (BlockListValidator, "stats_foo_three", True),
+            (BlockListValidator, "stats_three", True),
+        ],
+    )
+    def test_allow_and_block_list(self, validator, stat_name, expect_incr):
+        statsd_client = Mock(spec=statsd.StatsClient)
+        stats = SafeStatsdLogger(statsd_client, validator("stats_one, 
stats_two, foo"))
+
+        stats.incr(stat_name)
+
+        if expect_incr:
+            statsd_client.incr.assert_called_once_with(stat_name, 1, 1)
+        else:
+            statsd_client.assert_not_called()
+
+    @pytest.mark.parametrize(
+        "match_pattern, expect_incr",
+        [
+            ("^stat", True),
+            ("a.{4}o", True),
+            ("^banana", False),
+        ],
+    )
+    def test_regex_matches(self, match_pattern, expect_incr):
+        stat_name = "stats_foo_one"
+        validator = PatternAllowListValidator
 
-    def test_increment_counter_with_allowed_key(self):
-        self.stats.incr("stats_one")
-        self.statsd_client.incr.assert_called_once_with("stats_one", 1, 1)
+        statsd_client = Mock(spec=statsd.StatsClient)
+        stats = SafeStatsdLogger(statsd_client, validator(match_pattern))
 
-    def test_increment_counter_with_allowed_prefix(self):
-        self.stats.incr("stats_two.bla")
-        self.statsd_client.incr.assert_called_once_with("stats_two.bla", 1, 1)
+        stats.incr(stat_name)
 
-    def test_not_increment_counter_if_not_allowed(self):
-        self.stats.incr("stats_three")
-        self.statsd_client.assert_not_called()
+        if expect_incr:
+            statsd_client.incr.assert_called_once_with(stat_name, 1, 1)
+        else:
+            statsd_client.assert_not_called()
 
 
-class TestStatsWithBlockList:
-    def setup_method(self):
-        self.statsd_client = Mock(spec=statsd.StatsClient)
-        self.stats = SafeStatsdLogger(self.statsd_client, 
BlockListValidator("stats_one, stats_two"))
+class TestPatternOrBasicValidatorConfigOption:
+    def teardown_method(self):
+        # Avoid side-effects
+        importlib.reload(airflow.stats)
 
-    def test_increment_counter_with_allowed_key(self):
-        self.stats.incr("stats_one")
-        self.statsd_client.assert_not_called()
+    stats_on = {("metrics", "statsd_on"): "True"}
+    pattern_on = {("metrics", "metrics_use_pattern_match"): "True"}
+    pattern_off = {("metrics", "metrics_use_pattern_match"): "False"}
+    allow_list = {("metrics", "metrics_allow_list"): "foo,bar"}

Review Comment:
   Added a whole separate test using regex patterns.  This shows that you can 
just enter a string and it'll match anywhere in the name, and also optionally 
you can use regex patterns for the more complex stuff if you wish.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to