3857: code review feedback

[arvados.git] / sdk / python / tests / test_collections.py

diff --git a/sdk/python/tests/test_collections.py b/sdk/python/tests/test_collections.py
index 45067ad8a0e70e1d51f293b992cc9dddd500078e..fb29881223b08bd75649455df1739e972d52b027 100644 (file)
--- a/sdk/python/tests/test_collections.py
+++ b/sdk/python/tests/test_collections.py
@@ -5,9 +5,11 @@
 import arvados
 import bz2
 import copy
+import hashlib
 import mock
 import os
 import pprint
+import re
 import subprocess
 import tempfile
 import unittest
@@ -167,7 +169,7 @@ class ArvadosCollectionsTest(run_test_server.TestCaseWithServers,
         self.check_manifest_file_sizes(cw.manifest_text(), [1,0])
         self.check_manifest_file_sizes(
             arvados.CollectionReader(
-                cw.manifest_text()).normalize().manifest_text(),
+                cw.manifest_text()).manifest_text(normalize=True),
             [0,1])
 
     def check_manifest_file_sizes(self, manifest_text, expect_sizes):
@@ -229,19 +231,19 @@ class ArvadosCollectionsTest(run_test_server.TestCaseWithServers,
 . 085c37f02916da1cad16f93c54d899b7+41 0:41:md5sum.txt
 . 8b22da26f9f433dea0a10e5ec66d73ba+43 0:43:md5sum.txt
 """
-        self.assertEqual(arvados.CollectionReader(m1, self.api_client).normalize().manifest_text(),
+        self.assertEqual(arvados.CollectionReader(m1, self.api_client).manifest_text(normalize=True),
                          """. 5348b82a029fd9e971a811ce1f71360b+43 085c37f02916da1cad16f93c54d899b7+41 8b22da26f9f433dea0a10e5ec66d73ba+43 0:127:md5sum.txt
 """)
 
         m2 = """. 204e43b8a1185621ca55a94839582e6f+67108864 b9677abbac956bd3e86b1deb28dfac03+67108864 fc15aff2a762b13f521baf042140acec+67108864 323d2a3ce20370c4ca1d3462a344f8fd+25885655 0:227212247:var-GS000016015-ASM.tsv.bz2
 """
-        self.assertEqual(arvados.CollectionReader(m2, self.api_client).normalize().manifest_text(), m2)
+        self.assertEqual(arvados.CollectionReader(m2, self.api_client).manifest_text(normalize=True), m2)
 
         m3 = """. 5348b82a029fd9e971a811ce1f71360b+43 3:40:md5sum.txt
 . 085c37f02916da1cad16f93c54d899b7+41 0:41:md5sum.txt
 . 8b22da26f9f433dea0a10e5ec66d73ba+43 0:43:md5sum.txt
 """
-        self.assertEqual(arvados.CollectionReader(m3, self.api_client).normalize().manifest_text(),
+        self.assertEqual(arvados.CollectionReader(m3, self.api_client).manifest_text(normalize=True),
                          """. 5348b82a029fd9e971a811ce1f71360b+43 085c37f02916da1cad16f93c54d899b7+41 8b22da26f9f433dea0a10e5ec66d73ba+43 3:124:md5sum.txt
 """)
 
@@ -249,7 +251,7 @@ class ArvadosCollectionsTest(run_test_server.TestCaseWithServers,
 ./zzz 204e43b8a1185621ca55a94839582e6f+67108864 0:999:zzz
 ./foo 323d2a3ce20370c4ca1d3462a344f8fd+25885655 0:3:bar
 """
-        self.assertEqual(arvados.CollectionReader(m4, self.api_client).normalize().manifest_text(),
+        self.assertEqual(arvados.CollectionReader(m4, self.api_client).manifest_text(normalize=True),
                          """./foo 204e43b8a1185621ca55a94839582e6f+67108864 323d2a3ce20370c4ca1d3462a344f8fd+25885655 0:3:bar 67108864:3:bar
 ./zzz 204e43b8a1185621ca55a94839582e6f+67108864 0:999:zzz
 """)
@@ -258,22 +260,22 @@ class ArvadosCollectionsTest(run_test_server.TestCaseWithServers,
 ./zzz 204e43b8a1185621ca55a94839582e6f+67108864 0:999:zzz
 ./foo 204e43b8a1185621ca55a94839582e6f+67108864 3:3:bar
 """
-        self.assertEqual(arvados.CollectionReader(m5, self.api_client).normalize().manifest_text(),
+        self.assertEqual(arvados.CollectionReader(m5, self.api_client).manifest_text(normalize=True),
                          """./foo 204e43b8a1185621ca55a94839582e6f+67108864 0:6:bar
 ./zzz 204e43b8a1185621ca55a94839582e6f+67108864 0:999:zzz
 """)
 
         with self.data_file('1000G_ref_manifest') as f6:
             m6 = f6.read()
-            self.assertEqual(arvados.CollectionReader(m6, self.api_client).normalize().manifest_text(), m6)
+            self.assertEqual(arvados.CollectionReader(m6, self.api_client).manifest_text(normalize=True), m6)
 
         with self.data_file('jlake_manifest') as f7:
             m7 = f7.read()
-            self.assertEqual(arvados.CollectionReader(m7, self.api_client).normalize().manifest_text(), m7)
+            self.assertEqual(arvados.CollectionReader(m7, self.api_client).manifest_text(normalize=True), m7)
 
         m8 = """./a\\040b\\040c 59ca0efa9f5633cb0371bbc0355478d8+13 0:13:hello\\040world.txt
 """
-        self.assertEqual(arvados.CollectionReader(m8, self.api_client).normalize().manifest_text(), m8)
+        self.assertEqual(arvados.CollectionReader(m8, self.api_client).manifest_text(normalize=True), m8)
 
     def test_locators_and_ranges(self):
         blocks2 = [['a', 10, 0],
@@ -504,7 +506,7 @@ class ArvadosCollectionsTest(run_test_server.TestCaseWithServers,
 . 085c37f02916da1cad16f93c54d899b7+41 5348b82a029fd9e971a811ce1f71360b+43 8b22da26f9f433dea0a10e5ec66d73ba+43 40:80:md9sum.txt
 """
 
-        m2 = arvados.CollectionReader(m1, self.api_client).normalize().manifest_text()
+        m2 = arvados.CollectionReader(m1, self.api_client).manifest_text(normalize=True)
 
         self.assertEqual(m2,
                          ". 5348b82a029fd9e971a811ce1f71360b+43 085c37f02916da1cad16f93c54d899b7+41 8b22da26f9f433dea0a10e5ec66d73ba+43 0:43:md5sum.txt 43:41:md6sum.txt 84:43:md7sum.txt 6:37:md8sum.txt 84:43:md8sum.txt 83:1:md9sum.txt 0:43:md9sum.txt 84:36:md9sum.txt\n")
@@ -553,6 +555,17 @@ class ArvadosCollectionsTest(run_test_server.TestCaseWithServers,
                          """. bd19836ddb62c11c55ab251ccaca5645+2 0:2:f1
 ./d1 50170217e5b04312024aa5cd42934494+13 0:8:d2/f3 8:5:f2\n""")
 
+    def test_write_directory_tree_with_zero_recursion(self):
+        cwriter = arvados.CollectionWriter(self.api_client)
+        content = 'd1/d2/f3d1/f2f1'
+        blockhash = hashlib.md5(content).hexdigest() + '+' + str(len(content))
+        cwriter.write_directory_tree(
+            self.build_directory_tree(['f1', 'd1/f2', 'd1/d2/f3']),
+            max_manifest_depth=0)
+        self.assertEqual(
+            cwriter.manifest_text(),
+            ". {} 0:8:d1/d2/f3 8:5:d1/f2 13:2:f1\n".format(blockhash))
+
     def test_write_one_file(self):
         cwriter = arvados.CollectionWriter(self.api_client)
         with self.make_test_file() as testfile:
@@ -668,6 +681,9 @@ class CollectionTestMixin(object):
     DEFAULT_DATA_HASH = DEFAULT_COLLECTION['portable_data_hash']
     DEFAULT_MANIFEST = DEFAULT_COLLECTION['manifest_text']
     DEFAULT_UUID = DEFAULT_COLLECTION['uuid']
+    ALT_COLLECTION = API_COLLECTIONS['bar_file']
+    ALT_DATA_HASH = ALT_COLLECTION['portable_data_hash']
+    ALT_MANIFEST = ALT_COLLECTION['manifest_text']
 
     def _mock_api_call(self, mock_method, code, body):
         mock_method = mock_method().execute
@@ -720,7 +736,7 @@ class CollectionReaderTestCase(unittest.TestCase, CollectionTestMixin):
     def test_locator_init(self):
         client = self.api_client_mock(200)
         # Ensure Keep will not return anything if asked.
-        with tutil.mock_responses(None, 404):
+        with tutil.mock_get_responses(None, 404):
             reader = arvados.CollectionReader(self.DEFAULT_DATA_HASH,
                                               api_client=client)
             self.assertEqual(self.DEFAULT_MANIFEST, reader.manifest_text())
@@ -729,7 +745,8 @@ class CollectionReaderTestCase(unittest.TestCase, CollectionTestMixin):
         # crunch-job needs this to read manifests that have only ever
         # been written to Keep.
         client = self.api_client_mock(200)
-        with tutil.mock_responses(self.DEFAULT_MANIFEST, 404, 200):
+        self.mock_get_collection(client, 404, None)
+        with tutil.mock_get_responses(self.DEFAULT_MANIFEST, 200):
             reader = arvados.CollectionReader(self.DEFAULT_DATA_HASH,
                                               api_client=client)
             self.assertEqual(self.DEFAULT_MANIFEST, reader.manifest_text())
@@ -739,7 +756,7 @@ class CollectionReaderTestCase(unittest.TestCase, CollectionTestMixin):
         client = self.api_client_mock(404)
         reader = arvados.CollectionReader(self.DEFAULT_UUID,
                                           api_client=client)
-        with tutil.mock_responses(self.DEFAULT_MANIFEST, 200):
+        with tutil.mock_get_responses(self.DEFAULT_MANIFEST, 200):
             with self.assertRaises(arvados.errors.ApiError):
                 reader.manifest_text()
 
@@ -747,11 +764,11 @@ class CollectionReaderTestCase(unittest.TestCase, CollectionTestMixin):
         # To verify that CollectionReader tries Keep first here, we
         # mock API server to return the wrong data.
         client = self.api_client_mock(200)
-        with tutil.mock_responses(self.DEFAULT_MANIFEST, 200):
+        with tutil.mock_get_responses(self.ALT_MANIFEST, 200):
             self.assertEqual(
-                self.DEFAULT_MANIFEST,
+                self.ALT_MANIFEST,
                 arvados.CollectionReader(
-                    self.DEFAULT_DATA_HASH + '+Affffffffffffffffffffffffffffffffffffffff@fedcba98',
+                    self.ALT_DATA_HASH + '+Affffffffffffffffffffffffffffffffffffffff@fedcba98',
                     api_client=client).manifest_text())
 
     def test_init_num_retries_propagated(self):
@@ -759,7 +776,7 @@ class CollectionReaderTestCase(unittest.TestCase, CollectionTestMixin):
         client = self.api_client_mock(200)
         reader = arvados.CollectionReader(self.DEFAULT_UUID, api_client=client,
                                           num_retries=3)
-        with tutil.mock_responses('foo', 500, 500, 200):
+        with tutil.mock_get_responses('foo', 500, 500, 200):
             self.assertEqual('foo',
                              ''.join(f.read(9) for f in reader.all_files()))
 
@@ -767,26 +784,29 @@ class CollectionReaderTestCase(unittest.TestCase, CollectionTestMixin):
         # client should be able to use CollectionReader on a manifest without normalizing it
         client = self.api_client_mock(500)
         nonnormal = ". acbd18db4cc2f85cedef654fccc4a4d8+3+Aabadbadbee@abeebdee 0:3:foo.txt 1:0:bar.txt 0:3:foo.txt\n"
+        reader = arvados.CollectionReader(
+            nonnormal,
+            api_client=client, num_retries=0)
+        # Ensure stripped_manifest() doesn't mangle our manifest in
+        # any way other than stripping hints.
         self.assertEqual(
-            ". acbd18db4cc2f85cedef654fccc4a4d8+3 0:3:foo.txt 1:0:bar.txt 0:3:foo.txt\n",
-            arvados.CollectionReader(
-                nonnormal,
-                api_client=client, num_retries=0).stripped_manifest())
+            re.sub('\+[^\d\s\+]+', '', nonnormal),
+            reader.stripped_manifest())
+        # Ensure stripped_manifest() didn't mutate our reader.
+        self.assertEqual(nonnormal, reader.manifest_text())
+        # Ensure the files appear in the order given in the manifest.
         self.assertEqual(
             [[6, '.', 'foo.txt'],
              [0, '.', 'bar.txt']],
             [[f.size(), f.stream_name(), f.name()]
-             for f in
-             arvados.CollectionReader(
-                    nonnormal,
-                    api_client=client, num_retries=0).all_streams()[0].all_files()])
+             for f in reader.all_streams()[0].all_files()])
 
 
 @tutil.skip_sleep
 class CollectionWriterTestCase(unittest.TestCase, CollectionTestMixin):
     def mock_keep(self, body, *codes, **headers):
         headers.setdefault('x-keep-replicas-stored', 2)
-        return tutil.mock_responses(body, *codes, **headers)
+        return tutil.mock_put_responses(body, *codes, **headers)
 
     def foo_writer(self, **kwargs):
         api_client = self.api_client_mock()