changeset 35666:2c6ebd0c850e

lfs: remove internal url in test `test-lfs-test-server.t` refers to a FB internal domain and requires certain implementation (ex. set error code to 404) at that endpoint. Without any workaround, It should in theory error out like "Domain cannot be resolved". I don't know how Matt Harbison ran the test. This patch changes the test to only depend on `lfs-test-server`. Unfortunately the logic has to be changed since `lfs-test-server` does not set error code to 404 but just removes "download" from "actions". Differential Revision: https://phab.mercurial-scm.org/D1849
author Jun Wu <quark@fb.com>
date Wed, 10 Jan 2018 21:27:05 -0800
parents 1ad1e59b405e
children de598e84c244
files hgext/lfs/blobstore.py tests/test-lfs-test-server.t
diffstat 2 files changed, 30 insertions(+), 18 deletions(-) [+]
line wrap: on
line diff
--- a/hgext/lfs/blobstore.py	Sun Jan 14 18:12:51 2018 -0500
+++ b/hgext/lfs/blobstore.py	Wed Jan 10 21:27:05 2018 -0800
@@ -227,20 +227,27 @@
                                  % rawjson)
         return response
 
-    def _checkforservererror(self, pointers, responses):
+    def _checkforservererror(self, pointers, responses, action):
         """Scans errors from objects
 
         Returns LfsRemoteError if any objects has an error"""
         for response in responses:
-            error = response.get('error')
-            if error:
+            # The server should return 404 when objects cannot be found. Some
+            # server implementation (ex. lfs-test-server)  does not set "error"
+            # but just removes "download" from "actions". Treat that case
+            # as the same as 404 error.
+            notfound = (response.get('error', {}).get('code') == 404
+                        or (action == 'download'
+                            and action not in response.get('actions', [])))
+            if notfound:
                 ptrmap = {p.oid(): p for p in pointers}
                 p = ptrmap.get(response['oid'], None)
-                if error['code'] == 404 and p:
+                if p:
                     filename = getattr(p, 'filename', 'unknown')
                     raise LfsRemoteError(
                         _(('LFS server error. Remote object '
                           'for "%s" not found: %r')) % (filename, response))
+            if 'error' in response:
                 raise LfsRemoteError(_('LFS server error: %r') % response)
 
     def _extractobjects(self, response, pointers, action):
@@ -252,21 +259,11 @@
         """
         # Scan errors from objects - fail early
         objects = response.get('objects', [])
-        self._checkforservererror(pointers, objects)
+        self._checkforservererror(pointers, objects, action)
 
         # Filter objects with given action. Practically, this skips uploading
         # objects which exist in the server.
         filteredobjects = [o for o in objects if action in o.get('actions', [])]
-        # But for downloading, we want all objects. Therefore missing objects
-        # should be considered an error.
-        if action == 'download':
-            if len(filteredobjects) < len(objects):
-                missing = [o.get('oid', '?')
-                           for o in objects
-                           if action not in o.get('actions', [])]
-                raise LfsRemoteError(
-                    _('LFS server claims required objects do not exist:\n%s')
-                    % '\n'.join(missing))
 
         return filteredobjects
 
--- a/tests/test-lfs-test-server.t	Sun Jan 14 18:12:51 2018 -0500
+++ b/tests/test-lfs-test-server.t	Wed Jan 10 21:27:05 2018 -0800
@@ -160,12 +160,12 @@
   $ rm -rf .hg/store/lfs
   $ rm -rf `hg config lfs.usercache`
   $ hg update -C '.^'
-  abort: LFS server claims required objects do not exist:
-  8e6ea5f6c066b44a0efa43bcce86aea73f17e6e23f0663df0251e7524e140a13!
+  abort: LFS server error. Remote object for "b" not found:(.*)! (re)
   [255]
 
 Check error message when object does not exist:
 
+  $ cd $TESTTMP
   $ hg init test && cd test
   $ echo "[extensions]" >> .hg/hgrc
   $ echo "lfs=" >> .hg/hgrc
@@ -183,7 +183,22 @@
   x-is-binary 0
   $ cd ..
   $ rm -rf `hg config lfs.usercache`
-  $ hg --config 'lfs.url=https://dewey-lfs.vip.facebook.com/lfs' clone test test2
+
+(Restart the server in a different location so it no longer has the content)
+
+  $ $PYTHON $RUNTESTDIR/killdaemons.py $DAEMON_PIDS
+  $ rm $DAEMON_PIDS
+  $ mkdir $TESTTMP/lfs-server2
+  $ cd $TESTTMP/lfs-server2
+#if no-windows
+  $ lfs-test-server &> lfs-server.log &
+  $ echo $! >> $DAEMON_PIDS
+#else
+  $ $PYTHON $TESTTMP/spawn.py >> $DAEMON_PIDS
+#endif
+
+  $ cd $TESTTMP
+  $ hg clone test test2
   updating to branch default
   abort: LFS server error. Remote object for "a" not found:(.*)! (re)
   [255]