Repository: iterative/dvc

Issue:
state: investigate excessive checksum re-calculations
From discord:

> I added the file, which computed checksum.  Then I dvc pushed, which computed checksum.  Then I dvc run -d thatfile, which computed checksum after the command finished.  Then I dvc pushed again (to make sure it wasn't going to push the dir that I untarred), which is computing the checksum again.


Discussion:
pared: @shcheklein sure, I'm on it.
mroutis: @shcheklein, don't hesitate to request changes with the GitHub interface :)
pared: @shcheklein how about now? :)

shcheklein: @pared this is great! thanks :) 👍 

Patch:
diff --git a/dvc/state.py b/dvc/state.py
index b2ca2a3cd..836b59ec3 100644
--- a/dvc/state.py
+++ b/dvc/state.py
@@ -263,8 +263,9 @@ class State(object):
                     m = st.st_mtime
                     if m > mtime:
                         mtime = m
-
-        return (str(int(nanotime.timestamp(mtime))), size)
+        # State of files handled by dvc is stored in db as TEXT.
+        # We cast results to string for later comparisons with stored values.
+        return str(int(nanotime.timestamp(mtime))), str(size)
 
     @staticmethod
     def inode(path):


Test patch:
diff --git a/tests/test_data_cloud.py b/tests/test_data_cloud.py
index d13eb4040..f9cdb8415 100644
--- a/tests/test_data_cloud.py
+++ b/tests/test_data_cloud.py
@@ -7,6 +7,7 @@ import shutil
 import getpass
 import platform
 import yaml
+from dvc.state import State
 from mock import patch
 
 import dvc.logger as logger
@@ -18,6 +19,7 @@ from dvc.remote.base import STATUS_OK, STATUS_NEW, STATUS_DELETED
 from dvc.utils import file_md5
 
 from tests.basic_env import TestDvc
+from tests.utils import spy
 
 try:
     from StringIO import StringIO
@@ -684,3 +686,16 @@ class TestRecursiveSyncOperations(TestDataCloudBase):
         self._test_recursive_fetch(data_md5, data_sub_md5)
 
         self._test_recursive_pull()
+
+
+class TestCheckSumRecalculation(TestDvc):
+
+    def test(self):
+        test_collect = spy(State._collect)
+        with patch.object(State, '_collect', test_collect):
+            url = get_local_url()
+            main(['remote', 'add', '-d', TEST_REMOTE, url])
+            main(['add', self.FOO])
+            main(['push'])
+            main(['run', '-d', self.FOO, 'echo foo'])
+        self.assertEqual(test_collect.mock.call_count, 1)
diff --git a/tests/unit/state.py b/tests/unit/state.py
new file mode 100644
index 000000000..b1608df10
--- /dev/null
+++ b/tests/unit/state.py
@@ -0,0 +1,13 @@
+from dvc.state import State
+from tests.basic_env import TestDvc
+
+
+class TestMtimeAndSizeReturnType(TestDvc):
+    def test(self):
+        file_time, file_size = State.mtime_and_size(self.DATA)
+        dir_time, dir_size = State.mtime_and_size(self.DATA_DIR)
+
+        self.assertIs(type(file_time), str)
+        self.assertIs(type(file_size), str)
+        self.assertIs(type(dir_time), str)
+        self.assertIs(type(dir_size), str)
diff --git a/tests/utils/__init__.py b/tests/utils/__init__.py
index e69de29bb..0867f7924 100644
--- a/tests/utils/__init__.py
+++ b/tests/utils/__init__.py
@@ -0,0 +1,12 @@
+from mock import MagicMock
+
+
+def spy(method_to_decorate):
+    mock = MagicMock()
+
+    def wrapper(self, *args, **kwargs):
+        mock(*args, **kwargs)
+        return method_to_decorate(self, *args, **kwargs)
+
+    wrapper.mock = mock
+    return wrapper