From 4371587193e16adc748c87418d3bdd59cbf9999a Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 26 Mar 2019 16:29:44 -0700 Subject: [PATCH 1/2] Add ability to inject statsd client; some py test/reqs updates - Updated the metrics logger to allow construction with an existing statsd client, so that it can be configured by external systems or libs. - added requirements to requirements-dev.txt which are needed to run tests-eg coverage, nose - removed dependency on mock lib, it is in python stdlib now - updated tox.ini to remove the now-superfluous deps --- requirements-dev.txt | 2 ++ superset/stats_logger.py | 15 +++++++++++++-- tests/access_tests.py | 3 +-- tests/base_tests.py | 2 +- tests/core_tests.py | 2 +- tests/db_engine_specs_test.py | 2 +- tests/druid_func_tests.py | 2 +- tests/druid_tests.py | 3 +-- tests/email_tests.py | 3 +-- tests/schedules_test.py | 2 +- tests/stats_logger_tests.py | 34 ++++++++++++++++++++++++++++++++++ tests/utils_tests.py | 2 +- tests/viz_tests.py | 2 +- tox.ini | 3 --- 14 files changed, 59 insertions(+), 18 deletions(-) create mode 100644 tests/stats_logger_tests.py diff --git a/requirements-dev.txt b/requirements-dev.txt index 7e0d6a43216d6..1ef6617b5de87 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -15,6 +15,7 @@ # limitations under the License. # console_log==0.2.10 +coverage==4.5.3 flake8-commas==2.0.0 flake8-import-order==0.18 flake8-mypy==17.8.0 @@ -24,6 +25,7 @@ flask-cors==3.0.6 ipdb==0.11 mypy==0.670 mysqlclient==1.3.13 +nose==1.3.7 pip-tools==3.5.0 psycopg2-binary==2.7.5 pycodestyle==2.4.0 diff --git a/superset/stats_logger.py b/superset/stats_logger.py index ce4b40033ac92..3ab63133e58eb 100644 --- a/superset/stats_logger.py +++ b/superset/stats_logger.py @@ -73,8 +73,19 @@ def gauge(self, key, value): from statsd import StatsClient class StatsdStatsLogger(BaseStatsLogger): - def __init__(self, host, port, prefix='superset'): - self.client = StatsClient(host=host, port=port, prefix=prefix) + + def __init__(self, host='localhost', port=8125, + prefix='superset', statsd_client=None): + """ + Initializes from either params or a supplied, pre-constructed statsd client. + + If statsd_client argument is given, all other arguments are ignored and the + supplied client will be used to emit metrics. + """ + if statsd_client: + self.client = statsd_client + else: + self.client = StatsClient(host=host, port=port, prefix=prefix) def incr(self, key): self.client.incr(key) diff --git a/tests/access_tests.py b/tests/access_tests.py index f3634014767df..f608f00a3b25d 100644 --- a/tests/access_tests.py +++ b/tests/access_tests.py @@ -17,8 +17,7 @@ """Unit tests for Superset""" import json import unittest - -import mock +from unittest import mock from superset import app, db, security_manager from superset.connectors.connector_registry import ConnectorRegistry diff --git a/tests/base_tests.py b/tests/base_tests.py index 8884bbb0a1d4e..8555915fd0fe5 100644 --- a/tests/base_tests.py +++ b/tests/base_tests.py @@ -17,9 +17,9 @@ """Unit tests for Superset""" import json import unittest +from unittest.mock import Mock, patch from flask_appbuilder.security.sqla import models as ab_models -from mock import Mock, patch import pandas as pd from superset import app, db, is_feature_enabled, security_manager diff --git a/tests/core_tests.py b/tests/core_tests.py index 14a6ed80f2073..00e83ace58fae 100644 --- a/tests/core_tests.py +++ b/tests/core_tests.py @@ -26,8 +26,8 @@ import re import string import unittest +from unittest import mock -import mock import pandas as pd import psycopg2 import sqlalchemy as sqla diff --git a/tests/db_engine_specs_test.py b/tests/db_engine_specs_test.py index b5d591e52194d..e1286076bb623 100644 --- a/tests/db_engine_specs_test.py +++ b/tests/db_engine_specs_test.py @@ -15,8 +15,8 @@ # specific language governing permissions and limitations # under the License. import inspect +from unittest import mock -import mock from sqlalchemy import column, select, table from sqlalchemy.dialects.mssql import pymssql from sqlalchemy.types import String, UnicodeText diff --git a/tests/druid_func_tests.py b/tests/druid_func_tests.py index d2e9edc02596c..cf0c5e99fc925 100644 --- a/tests/druid_func_tests.py +++ b/tests/druid_func_tests.py @@ -16,8 +16,8 @@ # under the License. import json import unittest +from unittest.mock import Mock -from mock import Mock from pydruid.utils.dimensions import MapLookupExtraction, RegexExtraction import pydruid.utils.postaggregator as postaggs diff --git a/tests/druid_tests.py b/tests/druid_tests.py index ab26a98e57914..78d1bb8635d2b 100644 --- a/tests/druid_tests.py +++ b/tests/druid_tests.py @@ -18,8 +18,7 @@ from datetime import datetime import json import unittest - -from mock import Mock, patch +from unittest.mock import Mock, patch from superset import db, security_manager from superset.connectors.druid.models import ( diff --git a/tests/email_tests.py b/tests/email_tests.py index 163d1c30f4ba5..7e1830ccac5b3 100644 --- a/tests/email_tests.py +++ b/tests/email_tests.py @@ -22,8 +22,7 @@ import logging import tempfile import unittest - -import mock +from unittest import mock from superset import app from superset.utils import core as utils diff --git a/tests/schedules_test.py b/tests/schedules_test.py index 858cb7eb7e1ca..b0d311ab89e9a 100644 --- a/tests/schedules_test.py +++ b/tests/schedules_test.py @@ -16,9 +16,9 @@ # under the License. from datetime import datetime, timedelta import unittest +from unittest.mock import Mock, patch, PropertyMock from flask_babel import gettext as __ -from mock import Mock, patch, PropertyMock from selenium.common.exceptions import WebDriverException from superset import app, db diff --git a/tests/stats_logger_tests.py b/tests/stats_logger_tests.py new file mode 100644 index 0000000000000..a7d840184d98a --- /dev/null +++ b/tests/stats_logger_tests.py @@ -0,0 +1,34 @@ +from unittest import TestCase +from unittest.mock import Mock, patch + +from superset.stats_logger import StatsdStatsLogger + + +class StatsdStatsLoggerTest(TestCase): + + def verify_client_calls(self, logger, client): + logger.incr('foo1') + client.incr.assert_called_once() + client.incr.assert_called_with('foo1') + logger.decr('foo2') + client.decr.assert_called_once() + client.decr.assert_called_with('foo2') + logger.gauge('foo3') + client.gauge.assert_called_once() + client.gauge.assert_called_with('foo3') + logger.timing('foo4', 1.234) + client.timing.assert_called_once() + client.timing.assert_called_with('foo4', 1.234) + + def test_init_with_statsd_client(self): + client = Mock() + stats_logger = StatsdStatsLogger(statsd_client=client) + self.verify_client_calls(stats_logger, client) + + def test_init_with_params(self): + with patch('superset.stats_logger.StatsClient') as MockStatsdClient: + mock_client = MockStatsdClient.return_value + + stats_logger = StatsdStatsLogger() + self.verify_client_calls(stats_logger, mock_client) + diff --git a/tests/utils_tests.py b/tests/utils_tests.py index 587e07fc83d17..7e2908984f424 100644 --- a/tests/utils_tests.py +++ b/tests/utils_tests.py @@ -17,9 +17,9 @@ from datetime import date, datetime, time, timedelta from decimal import Decimal import unittest +from unittest.mock import patch import uuid -from mock import patch import numpy from superset.exceptions import SupersetException diff --git a/tests/viz_tests.py b/tests/viz_tests.py index b84c66184e7ba..facb8c3a1525d 100644 --- a/tests/viz_tests.py +++ b/tests/viz_tests.py @@ -15,9 +15,9 @@ # specific language governing permissions and limitations # under the License. from datetime import datetime +from unittest.mock import Mock, patch import uuid -from mock import Mock, patch import pandas as pd from superset import app diff --git a/tox.ini b/tox.ini index cf1a84f79c9e2..5eb9db5df7624 100644 --- a/tox.ini +++ b/tox.ini @@ -51,9 +51,6 @@ commands = deps = -rrequirements.txt -rrequirements-dev.txt - coverage - mock - nose setenv = PYTHONPATH = {toxinidir} SUPERSET_CONFIG = tests.superset_test_config From d862804287b08f5ab955847fc806b18d3a55be74 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 26 Mar 2019 18:36:14 -0700 Subject: [PATCH 2/2] add license to test file, and remove blank line at EOF --- tests/stats_logger_tests.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tests/stats_logger_tests.py b/tests/stats_logger_tests.py index a7d840184d98a..531c136bcbbdf 100644 --- a/tests/stats_logger_tests.py +++ b/tests/stats_logger_tests.py @@ -1,3 +1,20 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Unit tests for Superset""" from unittest import TestCase from unittest.mock import Mock, patch @@ -31,4 +48,3 @@ def test_init_with_params(self): stats_logger = StatsdStatsLogger() self.verify_client_calls(stats_logger, mock_client) -