From dd17ee0dbe8e3bd66a02a1da2ccaeef182b7e972 Mon Sep 17 00:00:00 2001 From: Sharon Goliath Date: Mon, 26 Aug 2024 17:45:55 -0700 Subject: [PATCH 1/3] CADC-13477 - do not use the fully-qualified name, when changing directories, in case the working directory is ./ --- caom2utils/caom2utils/data_util.py | 6 ++--- caom2utils/caom2utils/tests/test_data_util.py | 27 +++++++------------ caom2utils/setup.cfg | 2 +- 3 files changed, 13 insertions(+), 22 deletions(-) diff --git a/caom2utils/caom2utils/data_util.py b/caom2utils/caom2utils/data_util.py index c59ae007..4f5f429a 100644 --- a/caom2utils/caom2utils/data_util.py +++ b/caom2utils/caom2utils/data_util.py @@ -192,8 +192,8 @@ def put(self, working_directory, uri): fqn = path.join(working_directory, f_name) chdir(working_directory) try: - local_meta = get_local_file_info(fqn) - encoding = get_file_encoding(fqn) + local_meta = get_local_file_info(f_name) + encoding = get_file_encoding(f_name) replace = True cadc_meta = self.info(uri) if cadc_meta is None: @@ -204,7 +204,7 @@ def put(self, working_directory, uri): ) self._cadc_client.cadcput( uri, - src=fqn, + src=f_name, replace=replace, file_type=local_meta.file_type, file_encoding=encoding, diff --git a/caom2utils/caom2utils/tests/test_data_util.py b/caom2utils/caom2utils/tests/test_data_util.py index 68748a6c..e5943a6c 100644 --- a/caom2utils/caom2utils/tests/test_data_util.py +++ b/caom2utils/caom2utils/tests/test_data_util.py @@ -251,24 +251,15 @@ def _check_info_result(result): def _check_put_result(client_mock): assert client_mock.called, 'expect put mock call' - try: - client_mock.assert_called_with( - 'TEST', - 'test_file.fits', - archive_stream='default', - mime_type='application/fits', - mime_encoding=None, - md5_check=True, - ), 'wrong put args call' - except AssertionError: - client_mock.assert_called_with( - 'cadc:TEST/test_file.fits', - src=f'{test_fits2caom2.TESTDATA_DIR}/test_file.fits', - replace=True, - file_type='application/fits', - file_encoding=None, - md5_checksum='3c66ee2cb6e0c2cfb5cd6824d353dc11', - ) + client_mock.assert_called_with( + 'cadc:TEST/test_file.fits', + # src=f'{test_fits2caom2.TESTDATA_DIR}/test_file.fits', + src='test_file.fits', + replace=True, + file_type='application/fits', + file_encoding=None, + md5_checksum='3c66ee2cb6e0c2cfb5cd6824d353dc11', + ) def _fail_mock(test_wrapper, test_uri, test_working_directory): diff --git a/caom2utils/setup.cfg b/caom2utils/setup.cfg index a18ac79a..02fff034 100644 --- a/caom2utils/setup.cfg +++ b/caom2utils/setup.cfg @@ -33,7 +33,7 @@ url = https://www.cadc-ccda.hia-iha.nrc-cnrc.gc.ca/caom2 edit_on_github = False github_project = opencadc/caom2tools # version should be PEP386 compatible (http://www.python.org/dev/peps/pep-0386) -version = 1.7.3 +version = 1.7.4 [options] install_requires = From bf671c8b1e75b32333ad26a4462309d594de558e Mon Sep 17 00:00:00 2001 From: Sharon Goliath Date: Thu, 5 Sep 2024 10:59:57 -0700 Subject: [PATCH 2/3] CADC-13477 - add the test case that exposed the bug. --- caom2utils/caom2utils/tests/test_data_util.py | 103 +++++++++--------- 1 file changed, 52 insertions(+), 51 deletions(-) diff --git a/caom2utils/caom2utils/tests/test_data_util.py b/caom2utils/caom2utils/tests/test_data_util.py index e5943a6c..67e08a7d 100644 --- a/caom2utils/caom2utils/tests/test_data_util.py +++ b/caom2utils/caom2utils/tests/test_data_util.py @@ -107,62 +107,64 @@ def test_get_file_type(): def test_storage_inventory_client(cadc_client_mock): test_subject = Mock(autospec=True) test_uri = 'cadc:TEST/test_file.fits' - test_working_directory = Path(test_fits2caom2.TESTDATA_DIR) - test_fqn = test_working_directory / 'test_file.fits' - if test_fqn.exists(): - test_fqn.unlink() - - def info_si_mock(ignore): - return FileInfo(id=test_uri, file_type='application/fits', md5sum='abc', size=42) - - def get_si_mock(ignore2, dest, **kwargs): - fhead = kwargs.get('fhead') - if fhead: - dest.write(TEST_HEADERS) - else: - test_fqn.write_text('StorageInventoryClient') - - cadc_client_mock.return_value.cadcinfo.side_effect = info_si_mock - cadc_client_mock.return_value.cadcget.side_effect = get_si_mock - cadc_client_mock.return_value.cadcput = Mock(autospec=True) - cadc_client_mock.return_value.cadcremove = Mock(autospec=True) - - test_wrapper = data_util.StorageClientWrapper(subject=test_subject) - assert test_wrapper is not None, 'ctor failure' - - # info - test_result = test_wrapper.info(test_uri) - _check_info_result(test_result) - - # get_head - test_result = test_wrapper.get_head(test_uri) - _check_header_result(test_result) + for test_working_directory in [Path(test_fits2caom2.TESTDATA_DIR), Path('./')]: + test_fqn = test_working_directory / 'test_file.fits' + if test_fqn.exists(): + test_fqn.unlink() + + def info_si_mock(ignore): + return FileInfo(id=test_uri, file_type='application/fits', md5sum='abc', size=42) + + def get_si_mock(ignore2, dest, **kwargs): + fhead = kwargs.get('fhead') + if fhead: + dest.write(TEST_HEADERS) + else: + test_fqn.write_text('StorageInventoryClient') + + cadc_client_mock.return_value.cadcinfo.side_effect = info_si_mock + cadc_client_mock.return_value.cadcget.side_effect = get_si_mock + cadc_client_mock.return_value.cadcput = Mock(autospec=True) + cadc_client_mock.return_value.cadcremove = Mock(autospec=True) + + test_wrapper = data_util.StorageClientWrapper(subject=test_subject) + assert test_wrapper is not None, 'ctor failure' + + # info + test_result = test_wrapper.info(test_uri) + _check_info_result(test_result) + + # get_head + test_result = test_wrapper.get_head(test_uri) + _check_header_result(test_result) + + # get + test_wrapper.get(test_working_directory, test_uri) + _check_get_result(test_fqn) - # get - test_wrapper.get(test_working_directory, test_uri) - _check_get_result(test_fqn) + # put + test_wrapper.put(test_working_directory, test_uri) + _check_put_result(cadc_client_mock.return_value.cadcput) - # put - test_wrapper.put(test_working_directory, test_uri) - _check_put_result(cadc_client_mock.return_value.cadcput) + # delete + test_wrapper.remove(test_uri) + assert cadc_client_mock.return_value.cadcremove.called, 'remove call' + cadc_client_mock.return_value.cadcremove.assert_called_with(test_uri), 'wrong remove args' - # delete - test_wrapper.remove(test_uri) - assert cadc_client_mock.return_value.cadcremove.called, 'remove call' - cadc_client_mock.return_value.cadcremove.assert_called_with(test_uri), 'wrong remove args' + cadc_client_mock.return_value.cadcinfo.side_effect = exceptions.UnexpectedException('cadcinfo') + cadc_client_mock.return_value.cadcget.side_effect = exceptions.UnexpectedException('cadcget') + cadc_client_mock.return_value.cadcput.side_effect = exceptions.UnexpectedException('cadcput') + _fail_mock(test_wrapper, test_uri, test_working_directory) - cadc_client_mock.return_value.cadcinfo.side_effect = exceptions.UnexpectedException('cadcinfo') - cadc_client_mock.return_value.cadcget.side_effect = exceptions.UnexpectedException('cadcget') - cadc_client_mock.return_value.cadcput.side_effect = exceptions.UnexpectedException('cadcput') - _fail_mock(test_wrapper, test_uri, test_working_directory) + cadc_client_mock.return_value.cadcremove.side_effect = exceptions.UnexpectedException('cadcremove') + with pytest.raises(exceptions.UnexpectedException): + test_wrapper.remove(test_uri) - cadc_client_mock.return_value.cadcremove.side_effect = exceptions.UnexpectedException('cadcremove') - with pytest.raises(exceptions.UnexpectedException): - test_wrapper.remove(test_uri) + cadc_client_mock.return_value.cadcinfo.side_effect = exceptions.NotFoundException('cadcinfo') + test_result = test_wrapper.info(test_uri) + assert test_result is None, 'expected when not found' - cadc_client_mock.return_value.cadcinfo.side_effect = exceptions.NotFoundException('cadcinfo') - test_result = test_wrapper.info(test_uri) - assert test_result is None, 'expected when not found' + cadc_client_mock.reset() @patch('caom2utils.data_util.StorageInventoryClient') @@ -253,7 +255,6 @@ def _check_put_result(client_mock): assert client_mock.called, 'expect put mock call' client_mock.assert_called_with( 'cadc:TEST/test_file.fits', - # src=f'{test_fits2caom2.TESTDATA_DIR}/test_file.fits', src='test_file.fits', replace=True, file_type='application/fits', From b7679c10ced5431680b805f194f5c114c09821af Mon Sep 17 00:00:00 2001 From: Sharon Goliath Date: Mon, 16 Sep 2024 11:28:39 -0700 Subject: [PATCH 3/3] CADC-13477 - code review comments. --- caom2utils/caom2utils/tests/test_data_util.py | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/caom2utils/caom2utils/tests/test_data_util.py b/caom2utils/caom2utils/tests/test_data_util.py index 67e08a7d..cbb5ad04 100644 --- a/caom2utils/caom2utils/tests/test_data_util.py +++ b/caom2utils/caom2utils/tests/test_data_util.py @@ -107,21 +107,22 @@ def test_get_file_type(): def test_storage_inventory_client(cadc_client_mock): test_subject = Mock(autospec=True) test_uri = 'cadc:TEST/test_file.fits' + + def info_si_mock(ignore): + return FileInfo(id=test_uri, file_type='application/fits', md5sum='abc', size=42) + + def get_si_mock(ignore2, dest, **kwargs): + fhead = kwargs.get('fhead') + if fhead: + dest.write(TEST_HEADERS) + else: + test_fqn.write_text('StorageInventoryClient') + for test_working_directory in [Path(test_fits2caom2.TESTDATA_DIR), Path('./')]: test_fqn = test_working_directory / 'test_file.fits' if test_fqn.exists(): test_fqn.unlink() - def info_si_mock(ignore): - return FileInfo(id=test_uri, file_type='application/fits', md5sum='abc', size=42) - - def get_si_mock(ignore2, dest, **kwargs): - fhead = kwargs.get('fhead') - if fhead: - dest.write(TEST_HEADERS) - else: - test_fqn.write_text('StorageInventoryClient') - cadc_client_mock.return_value.cadcinfo.side_effect = info_si_mock cadc_client_mock.return_value.cadcget.side_effect = get_si_mock cadc_client_mock.return_value.cadcput = Mock(autospec=True)