Fixed tests

Author: wonder-sk

mergin/client.py

@@ -457,6 +457,10 @@ def apply_pull_changes(self, changes, temp_dir):
                         # We just apply the diff between our copy and server to both the local copy and its basefile
                         try:
                             server_diff = self.fpath(f'{path}-server_diff', temp_dir)  # diff between server file and local basefile
+                            # TODO: it could happen that basefile does not exist.
+                            # It was either never created (e.g. when pushing without geodiff)
+                            # or it was deleted by mistake(?) by the user. We should detect that
+                            # when starting pull and download it as well
                             self.geodiff.create_changeset(basefile, src, server_diff)
                             self.geodiff.apply_changeset(dest, server_diff)
                             self.geodiff.apply_changeset(basefile, server_diff)
@@ -1019,6 +1023,11 @@ def pull_project(self, directory, parallel=True):
         if local_version == server_info["version"]:
             return  # Project is up to date
 
+        # we either download a versioned file using diffs (strongly preferred),
+        # but if we don't have history with diffs (e.g. uploaded without diffs)
+        # then we just download the whole file
+        _pulling_file_with_diffs = lambda f: 'diffs' in f and len(f['diffs']) != 0
+
         temp_dir = mp.fpath_meta(f'fetch_{local_version}-{server_info["version"]}')
         os.makedirs(temp_dir, exist_ok=True)
         pull_changes = mp.get_pull_changes(server_info["files"])
@@ -1028,7 +1037,7 @@ def pull_project(self, directory, parallel=True):
             fetch_files.append(f)
         # extend fetch files download list with various version of diff files (if needed)
         for f in pull_changes["updated"]:
-            if 'diffs' in f:
+            if _pulling_file_with_diffs(f):
                 for diff in f['diffs']:
                     diff_file = copy.deepcopy(f)
                     for k, v in f['history'].items():
@@ -1047,7 +1056,7 @@ def pull_project(self, directory, parallel=True):
             with concurrent.futures.ThreadPoolExecutor() as executor:
                 futures_map = {}
                 for file in fetch_files:
-                    diff_only = 'diffs' in file
+                    diff_only = _pulling_file_with_diffs(f)
                     future = executor.submit(self._download_file, project_path, file, temp_dir, parallel, diff_only)
                     futures_map[future] = file
 
@@ -1060,13 +1069,13 @@ def pull_project(self, directory, parallel=True):
         else:
             for file in fetch_files:
                 # TODO check it does not fail, do some retry on ClientError
-                diff_only = 'diffs' in file
+                diff_only = _pulling_file_with_diffs(f)
                 self._download_file(project_path, file, temp_dir, parallel, diff_only)
 
         # make sure we can update geodiff reference files (aka. basefiles) with diffs or
         # download their full versions so we have them up-to-date for applying changes
         for file in pull_changes['updated']:
-            if 'diffs' not in file:
+            if not _pulling_file_with_diffs(f):
                 continue
             file['version'] = server_info['version']
             basefile = mp.fpath_meta(file['path'])

mergin/test/test_client.py

@@ -209,18 +209,17 @@ def test_ignore_files(mc):
 
 # (diffs size limit, push geodiff enabled, pull geodiff enabled)
 diff_test_scenarios = [
-    (1024, True, True),
-    (1024*1024, True, True),
-    (1024, True, False),
-    (1024, False, True),
-    (1024, False, False),
+    (True, True),
+    (True, False),
+    (False, True),
+    (False, False),
 ]
 
 
-@pytest.mark.parametrize("diffs_limit, push_geodiff_enabled, pull_geodiff_enabled", diff_test_scenarios)
-def test_sync_diff(mc, diffs_limit, push_geodiff_enabled, pull_geodiff_enabled):
+@pytest.mark.parametrize("push_geodiff_enabled, pull_geodiff_enabled", diff_test_scenarios)
+def test_sync_diff(mc, push_geodiff_enabled, pull_geodiff_enabled):
 
-    test_project = f'test_sync_diff_{diffs_limit}_{int(push_geodiff_enabled)}_{int(pull_geodiff_enabled)}'
+    test_project = f'test_sync_diff_push{int(push_geodiff_enabled)}_pull{int(pull_geodiff_enabled)}'
     project = API_USER + '/' + test_project
     project_dir = os.path.join(TMP_DIR, test_project)  # primary project dir for updates
     project_dir_2 = os.path.join(TMP_DIR, test_project + '_2')  # concurrent project dir with no changes
@@ -277,7 +276,6 @@ def test_sync_diff(mc, diffs_limit, push_geodiff_enabled, pull_geodiff_enabled):
         assert not os.path.exists(mp.fpath_meta('renamed.gpkg'))
 
     # pull project in different directory
-    os.environ['DIFFS_LIMIT_SIZE'] = str(diffs_limit)
     toggle_geodiff(pull_geodiff_enabled)
     mp2 = MerginProject(project_dir_2)
     mc.pull_project(project_dir_2)