From c71619f7d3ec01de2c5a9a517701ecf88381830e Mon Sep 17 00:00:00 2001 From: Peter Amstutz Date: Fri, 24 Aug 2018 12:12:47 -0400 Subject: [PATCH] 14075: Fix cwl unit tests Arvados-DCO-1.1-Signed-off-by: Peter Amstutz --- sdk/cwl/tests/test_container.py | 30 ++++++++++++++++++++++++++++-- sdk/cwl/tests/test_job.py | 18 ++++++++++-------- sdk/cwl/tests/test_submit.py | 16 ++++++++-------- sdk/python/arvados/commands/run.py | 11 ++++++++++- 4 files changed, 56 insertions(+), 19 deletions(-) diff --git a/sdk/cwl/tests/test_container.py b/sdk/cwl/tests/test_container.py index 2295e934ac..0209d2eba9 100644 --- a/sdk/cwl/tests/test_container.py +++ b/sdk/cwl/tests/test_container.py @@ -4,6 +4,7 @@ import arvados_cwl from arvados_cwl.arvdocker import arv_docker_clear_cache +import arvados.config import logging import mock import unittest @@ -21,6 +22,32 @@ if not os.getenv('ARVADOS_DEBUG'): logging.getLogger('arvados.arv-run').setLevel(logging.WARN) +class CollectionMock(object): + def __init__(self, vwdmock, *args, **kwargs): + self.vwdmock = vwdmock + self.count = 0 + + def open(self, *args, **kwargs): + self.count += 1 + return self.vwdmock.open(*args, **kwargs) + + def copy(self, *args, **kwargs): + self.count += 1 + self.vwdmock.copy(*args, **kwargs) + + def save_new(self, *args, **kwargs): + pass + + def __len__(self): + return self.count + + def portable_data_hash(self): + if self.count == 0: + return arvados.config.EMPTY_BLOCK_LOCATOR + else: + return "99999999999999999999999999999996+99" + + class TestContainer(unittest.TestCase): # The test passes no builder.resources @@ -215,8 +242,7 @@ class TestContainer(unittest.TestCase): runner.fs_access.get_collection.side_effect = get_collection_mock vwdmock = mock.MagicMock() - collection_mock.return_value = vwdmock - vwdmock.portable_data_hash.return_value = "99999999999999999999999999999996+99" + collection_mock.side_effect = lambda *args, **kwargs: CollectionMock(vwdmock, *args, **kwargs) tool = cmap({ "inputs": [], diff --git a/sdk/cwl/tests/test_job.py b/sdk/cwl/tests/test_job.py index 30930dd49a..1a3f6a9c8e 100644 --- a/sdk/cwl/tests/test_job.py +++ b/sdk/cwl/tests/test_job.py @@ -19,6 +19,7 @@ from schema_salad.ref_resolver import Loader from schema_salad.sourceline import cmap from .mock_discovery import get_rootDesc from .matcher import JsonDiffMatcher, StripYAMLComments +from .test_container import CollectionMock if not os.getenv('ARVADOS_DEBUG'): logging.getLogger('arvados.cwl-runner').setLevel(logging.WARN) @@ -342,7 +343,8 @@ class TestWorkflow(unittest.TestCase): tool, metadata = document_loader.resolve_ref("tests/wf/scatter2.cwl") metadata["cwlVersion"] = tool["cwlVersion"] - mockcollection().portable_data_hash.return_value = "99999999999999999999999999999999+118" + mockc = mock.MagicMock() + mockcollection.side_effect = lambda *args, **kwargs: CollectionMock(mockc, *args, **kwargs) arvtool = arvados_cwl.ArvadosWorkflow(runner, tool, work_api="jobs", avsc_names=avsc_names, basedir="", make_fs_access=make_fs_access, loader=document_loader, @@ -366,8 +368,8 @@ class TestWorkflow(unittest.TestCase): 'HOME': '$(task.outdir)', 'TMPDIR': '$(task.tmpdir)'}, 'task.vwd': { - 'workflow.cwl': '$(task.keep)/99999999999999999999999999999999+118/workflow.cwl', - 'cwl.input.yml': '$(task.keep)/99999999999999999999999999999999+118/cwl.input.yml' + 'workflow.cwl': '$(task.keep)/99999999999999999999999999999996+99/workflow.cwl', + 'cwl.input.yml': '$(task.keep)/99999999999999999999999999999996+99/cwl.input.yml' }, 'command': [u'cwltool', u'--no-container', u'--move-outputs', u'--preserve-entire-environment', u'workflow.cwl#main', u'cwl.input.yml'], 'task.stdout': 'cwl.output.json'}]}, @@ -384,8 +386,8 @@ class TestWorkflow(unittest.TestCase): ['docker_image_locator', 'in docker', 'arvados/jobs']], find_or_create=True) - mockcollection().open().__enter__().write.assert_has_calls([mock.call(subwf)]) - mockcollection().open().__enter__().write.assert_has_calls([mock.call( + mockc.open().__enter__().write.assert_has_calls([mock.call(subwf)]) + mockc.open().__enter__().write.assert_has_calls([mock.call( '''{ "fileblub": { "basename": "token.txt", @@ -428,7 +430,7 @@ class TestWorkflow(unittest.TestCase): tool, metadata = document_loader.resolve_ref("tests/wf/echo-wf.cwl") metadata["cwlVersion"] = tool["cwlVersion"] - mockcollection().portable_data_hash.return_value = "99999999999999999999999999999999+118" + mockcollection.side_effect = lambda *args, **kwargs: CollectionMock(mock.MagicMock(), *args, **kwargs) arvtool = arvados_cwl.ArvadosWorkflow(runner, tool, work_api="jobs", avsc_names=avsc_names, basedir="", make_fs_access=make_fs_access, loader=document_loader, @@ -452,8 +454,8 @@ class TestWorkflow(unittest.TestCase): 'HOME': '$(task.outdir)', 'TMPDIR': '$(task.tmpdir)'}, 'task.vwd': { - 'workflow.cwl': '$(task.keep)/99999999999999999999999999999999+118/workflow.cwl', - 'cwl.input.yml': '$(task.keep)/99999999999999999999999999999999+118/cwl.input.yml' + 'workflow.cwl': '$(task.keep)/99999999999999999999999999999996+99/workflow.cwl', + 'cwl.input.yml': '$(task.keep)/99999999999999999999999999999996+99/cwl.input.yml' }, 'command': [u'cwltool', u'--no-container', u'--move-outputs', u'--preserve-entire-environment', u'workflow.cwl#main', u'cwl.input.yml'], 'task.stdout': 'cwl.output.json'}]}, diff --git a/sdk/cwl/tests/test_submit.py b/sdk/cwl/tests/test_submit.py index f8b557f6cb..246d80e506 100644 --- a/sdk/cwl/tests/test_submit.py +++ b/sdk/cwl/tests/test_submit.py @@ -286,14 +286,14 @@ class TestSubmit(unittest.TestCase): 'manifest_text': '. 5bcc9fe8f8d5992e6cf418dc7ce4dbb3+16 0:16:blub.txt\n', 'replication_desired': None, - 'name': 'submit_tool.cwl dependencies', - }), ensure_unique_name=True), + 'name': 'submit_tool.cwl dependencies (5d373e7629203ce39e7c22af98a0f881+52)', + }), ensure_unique_name=False), mock.call(body=JsonDiffMatcher({ 'manifest_text': '. 979af1245a12a1fed634d4222473bfdc+16 0:16:blorp.txt\n', 'replication_desired': None, - 'name': 'submit_wf.cwl input', - }), ensure_unique_name=True), + 'name': 'submit_wf.cwl input (169f39d466a5438ac4a90e779bf750c7+53)', + }), ensure_unique_name=False), mock.call(body=JsonDiffMatcher({ 'manifest_text': '. 61df2ed9ee3eb7dd9b799e5ca35305fa+1217 0:1217:workflow.cwl\n', @@ -473,14 +473,14 @@ class TestSubmit(unittest.TestCase): 'manifest_text': '. 5bcc9fe8f8d5992e6cf418dc7ce4dbb3+16 0:16:blub.txt\n', 'replication_desired': None, - 'name': 'submit_tool.cwl dependencies', - }), ensure_unique_name=True), + 'name': 'submit_tool.cwl dependencies (5d373e7629203ce39e7c22af98a0f881+52)', + }), ensure_unique_name=False), mock.call(body=JsonDiffMatcher({ 'manifest_text': '. 979af1245a12a1fed634d4222473bfdc+16 0:16:blorp.txt\n', 'replication_desired': None, - 'name': 'submit_wf.cwl input', - }), ensure_unique_name=True)]) + 'name': 'submit_wf.cwl input (169f39d466a5438ac4a90e779bf750c7+53)', + }), ensure_unique_name=False)]) expect_container = copy.deepcopy(stubs.expect_container_spec) stubs.api.container_requests().create.assert_called_with( diff --git a/sdk/python/arvados/commands/run.py b/sdk/python/arvados/commands/run.py index ca51de1fe3..5063d75f2a 100644 --- a/sdk/python/arvados/commands/run.py +++ b/sdk/python/arvados/commands/run.py @@ -248,10 +248,19 @@ def uploadfiles(files, api, dry_run=False, num_retries=0, logger.info("Uploaded to %s (%s)", pdh, collection.manifest_locator()) except arvados.errors.ApiError as ae: tries -= 1 + if pdh is None: + # Something weird going on here, probably a collection + # with a conflicting name but wrong PDH. We won't + # able to reuse it but we still need to save our + # collection, so so save it with unique name. + logger.info("Name conflict on '%s', existing collection has an unexpected portable data hash", name_pdh) + collection.save_new(name=name_pdh, owner_uuid=project, ensure_unique_name=True) + pdh = collection.portable_data_hash() + logger.info("Uploaded to %s (%s)", pdh, collection.manifest_locator()) else: # empty collection pdh = collection.portable_data_hash() - assert (pdh == config.EMPTY_BLOCK_LOCATOR), "Empty collection portable_data_hash did not have expected locator" + assert (pdh == config.EMPTY_BLOCK_LOCATOR), "Empty collection portable_data_hash did not have expected locator, was %s" % pdh logger.info("Using empty collection %s", pdh) for c in files: -- 2.30.2