Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue while sending analytics report #6407

Merged
merged 2 commits into from
Aug 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion dvc/analytics.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def collect_and_send_report(args=None, return_code=None):

with tempfile.NamedTemporaryFile(delete=False, mode="w") as fobj:
json.dump(report, fobj)
daemon(["analytics", fobj.name])
daemon(["analytics", fobj.name])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!



def is_enabled():
Expand Down
39 changes: 38 additions & 1 deletion tests/func/test_analytics.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import os
from unittest import mock
from unittest.mock import call

from dvc.analytics import _scm_in_use
import pytest

from dvc.analytics import _scm_in_use, collect_and_send_report
from dvc.main import main
from dvc.repo import Repo

Expand All @@ -23,6 +26,40 @@ def test_main_analytics(mock_is_enabled, mock_report, tmp_dir, dvc):
assert mock_report.called


@pytest.fixture
def mock_daemon(mocker):
def func(argv):
return main(["daemon", *argv])

m = mocker.patch("dvc.daemon.daemon", mocker.MagicMock(side_effect=func))
yield m
Comment on lines +30 to +35
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having a conditional daemonizer would be good for testing and QA, rather than mocking it. But didn't want to introduce big change.



class ANY:
def __init__(self, expected_type):
self.expected_type = expected_type

def __repr__(self):
return "Any" + self.expected_type.__name__.capitalize()

def __eq__(self, other):
return isinstance(other, self.expected_type)


@mock.patch("requests.post")
def test_collect_and_send_report(mock_post, dvc, mock_daemon):
collect_and_send_report()

assert mock_daemon.call_count == 1
assert mock_post.call_count == 1
assert mock_post.call_args == call(
"https://analytics.dvc.org",
json=ANY(dict),
headers={"content-type": "application/json"},
timeout=5,
)


def test_scm_dvc_only(tmp_dir, dvc):
scm = _scm_in_use()
assert scm == "NoSCM"
Expand Down