From 1058df0f77fe0d0e8480ea723d40cb617c006e80 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Fri, 8 Nov 2019 03:30:57 +0200 Subject: [PATCH] analytics: don't forget to convert to bool --- dvc/analytics.py | 32 +++----------------------------- tests/func/test_analytics.py | 10 ++-------- tests/unit/test_analytics.py | 28 +++++++++++++++++++--------- 3 files changed, 24 insertions(+), 46 deletions(-) diff --git a/dvc/analytics.py b/dvc/analytics.py index d4914a33e..8b39fc31b 100644 --- a/dvc/analytics.py +++ b/dvc/analytics.py @@ -204,28 +204,9 @@ class Analytics(object): json.dump(self.info, fobj) return fobj.name - @staticmethod - def _is_enabled_config(config): - from dvc.config import Config - - core = config.config.get(Config.SECTION_CORE, {}) - return core.get(Config.SECTION_CORE_ANALYTICS, True) - - @staticmethod - def _get_current_config(): - from dvc.config import Config - from dvc.repo import Repo - from dvc.exceptions import NotDvcRepoError - - try: - dvc_dir = Repo.find_dvc_dir() - except NotDvcRepoError: - dvc_dir = None - - return Config(dvc_dir, validate=False) - @staticmethod def is_enabled(cmd=None): + from dvc.config import Config, to_bool from dvc.command.daemon import CmdDaemonBase if env2bool("DVC_TEST"): @@ -234,15 +215,8 @@ class Analytics(object): if isinstance(cmd, CmdDaemonBase): return False - config = ( - Analytics._get_current_config() - if cmd is None or not hasattr(cmd, "config") - else cmd.config - ) - - assert config is not None - - enabled = Analytics._is_enabled_config(config) + core = Config(validate=False).config.get(Config.SECTION_CORE, {}) + enabled = to_bool(core.get(Config.SECTION_CORE_ANALYTICS, "true")) logger.debug( "Analytics is {}.".format("enabled" if enabled else "disabled") ) diff --git a/tests/func/test_analytics.py b/tests/func/test_analytics.py index b8af2367b..b42a11727 100644 --- a/tests/func/test_analytics.py +++ b/tests/func/test_analytics.py @@ -30,24 +30,18 @@ class TestAnalytics(TestDir): self.assertNotEqual(a.info[a.PARAM_SYSTEM_INFO], {}) @mock.patch.object(os, "getenv", new=_clean_getenv) - @mock.patch( - "dvc.analytics.Analytics._is_enabled_config", return_value=True - ) @mock.patch("requests.post") - def test_send(self, mockpost, _): + def test_send(self, mockpost): ret = main(["daemon", "analytics", Analytics().dump(), "-v"]) self.assertEqual(ret, 0) self.assertTrue(mockpost.called) @mock.patch.object(os, "getenv", new=_clean_getenv) - @mock.patch( - "dvc.analytics.Analytics._is_enabled_config", return_value=True - ) @mock.patch.object( requests, "post", side_effect=requests.exceptions.RequestException() ) - def test_send_failed(self, mockpost, _): + def test_send_failed(self, mockpost): ret = main(["daemon", "analytics", Analytics().dump(), "-v"]) self.assertEqual(ret, 0) diff --git a/tests/unit/test_analytics.py b/tests/unit/test_analytics.py index 96209ce8f..9e7e2dab1 100644 --- a/tests/unit/test_analytics.py +++ b/tests/unit/test_analytics.py @@ -1,14 +1,24 @@ +import pytest + from dvc.analytics import Analytics -from dvc.config import Config -from dvc.repo import NotDvcRepoError -def test_broken_config(mocker, dvc_repo): - Config(dvc_repo.dvc_dir, validate=False).set("cache", "type", "unknown") +@pytest.mark.parametrize( + "config, result", + [ + ({}, True), + ({"analytics": "false"}, False), + ({"analytics": "true"}, True), + ({"unknown": "broken"}, True), + ({"analytics": "false", "unknown": "broken"}, False), + ], +) +def test_is_enabled(dvc_repo, config, result, monkeypatch): + configobj = dvc_repo.config._repo_config + configobj["core"] = config + configobj.write() - assert Analytics._get_current_config() + # reset DVC_TEST env var, which affects `is_enabled()` + monkeypatch.delenv("DVC_TEST") - with mocker.patch( - "dvc.repo.Repo.find_dvc_dir", side_effect=NotDvcRepoError(".") - ): - assert Analytics._get_current_config() + assert result == Analytics.is_enabled()