Skip to content

Commit

Permalink
Fix CSV writing on windows and reactivate tests (#350)
Browse files Browse the repository at this point in the history
  • Loading branch information
jessebrennan committed Sep 11, 2019
1 parent 14836bb commit 9a0f704
Show file tree
Hide file tree
Showing 4 changed files with 3 additions and 17 deletions.
2 changes: 1 addition & 1 deletion hca/dss/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -756,7 +756,7 @@ def _write_output_manifest(self):
fieldnames, source_manifest = self._parse_manifest(self.manifest)
if 'file_path' not in fieldnames:
fieldnames.append('file_path')
with atomic_write(output, overwrite=True) as f:
with atomic_write(output, overwrite=True, newline='') as f:
writer = csv.DictWriter(f, fieldnames, delimiter='\t', quoting=csv.QUOTE_NONE)
writer.writeheader()
for row in source_manifest:
Expand Down
2 changes: 1 addition & 1 deletion test/integration/dss/test_dss_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ def test_python_manifest_download(self):
cwd = os.getcwd()
os.chdir(work_dir)
try:
with open('manifest.tsv', 'w') as manifest:
with open('manifest.tsv', 'w', newline='') as manifest:
tsv = csv.DictWriter(manifest,
fieldnames=('bundle_uuid',
'bundle_version',
Expand Down
2 changes: 1 addition & 1 deletion test/tutorial/scripts/api/download_manifest_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

dss = DSSClient()

with open("manifest.tsv", "w") as manifest:
with open("manifest.tsv", "w", newline='') as manifest:
tsv = csv.DictWriter(
manifest,
fieldnames=(
Expand Down
14 changes: 0 additions & 14 deletions test/unit/test_dss_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,6 @@ def test_file_path_filestore_root(self):
parts = ManifestDownloadContext._file_path('abcdefghij', 'nested_filestore').split(os.sep)
self.assertEqual(parts, ['nested_filestore', '.hca', 'v2', 'files_1_3_2', 'a', 'bcd', 'ef', 'abcdefghij'])

@unittest.skipIf(os.name is 'nt', 'Unable to test on Windows') # TODO windows testing refactor
@patch('logging.Logger.warning')
@patch('hca.dss.DownloadContext._download_file', side_effect=_fake_download_file)
def test_manifest_download(self, download_func, warning_log):
Expand All @@ -190,7 +189,6 @@ def test_manifest_download(self, download_func, warning_log):
self._assert_all_files_downloaded()
self._assert_manifest_updated_with_paths('')

@unittest.skipIf(os.name is 'nt', 'Unable to test on Windows') # TODO windows testing refactor
def _test_download_dir(self, download_dir):
with patch('hca.dss.DownloadContext._download_file', side_effect=_fake_download_file) as download_func:
self.dss.download_manifest(self.manifest_file, 'aws', layout='none', download_dir=download_dir)
Expand All @@ -201,23 +199,18 @@ def _test_download_dir(self, download_dir):
self._assert_all_files_downloaded(prefix=download_dir)
self._assert_manifest_updated_with_paths(download_dir)

@unittest.skipIf(os.name is 'nt', 'Unable to test on Windows') # TODO windows testing refactor
def test_download_dir_empty(self):
self._test_download_dir('')

@unittest.skipIf(os.name is 'nt', 'Unable to test on Windows') # TODO windows testing refactor
def test_download_dir_dot(self):
self._test_download_dir('.')

@unittest.skipIf(os.name is 'nt', 'Unable to test on Windows') # TODO windows testing refactor
def test_download_dir(self):
self._test_download_dir('a_nested_dir')

@unittest.skipIf(os.name is 'nt', 'Unable to test on Windows') # TODO windows testing refactor
def test_download_dir_dot_dir(self):
self._test_download_dir(os.path.join('.', 'a_nested_dir'))

@unittest.skipIf(os.name is 'nt', 'Unable to test on Windows') # TODO windows testing refactor
@patch('logging.Logger.warning')
@patch('hca.dss.DownloadContext._download_file', side_effect=_fake_download_file)
def test_manifest_download_different_path(self, download_func, warning_log):
Expand All @@ -238,7 +231,6 @@ def test_manifest_download_different_path(self, download_func, warning_log):
self._assert_all_files_downloaded()
self._assert_manifest_updated_with_paths('')

@unittest.skipIf(os.name is 'nt', 'Unable to test on Windows') # TODO windows testing refactor
@patch('logging.Logger.warning')
@patch('hca.dss.DownloadContext._download_file', side_effect=_fake_download_file)
def test_manifest_download_partial(self, _, warning_log):
Expand Down Expand Up @@ -312,7 +304,6 @@ def _assert_all_files_downloaded(self, more_files=None, prefix=''):
more_files = bundle_files.union(more_files) if more_files else bundle_files
super(TestManifestDownloadBundle, self)._assert_all_files_downloaded(more_files=more_files, prefix=prefix)

@unittest.skipIf(os.name is 'nt', 'Unable to test on Windows') # TODO windows testing refactor
@patch('hca.dss.DSSClient.get_bundle')
@patch('hca.dss.DownloadContext._download_file', side_effect=_fake_download_file)
def test_manifest_download_bundle(self, _, mock_get_bundle):
Expand Down Expand Up @@ -344,19 +335,15 @@ def _assert_bundle_json(self, prefix=''):
actual_hash = hashlib.sha256(f.read()).hexdigest()
self.assertEqual(actual_hash, expected_hash)

@unittest.skipIf(os.name is 'nt', 'Unable to test on Windows') # TODO windows testing refactor
def test_download_dir_empty(self):
self._test_download_dir('')

@unittest.skipIf(os.name is 'nt', 'Unable to test on Windows') # TODO windows testing refactor
def test_download_dir_dot(self):
self._test_download_dir('.')

@unittest.skipIf(os.name is 'nt', 'Unable to test on Windows') # TODO windows testing refactor
def test_download_dir(self):
self._test_download_dir('a_nested_dir')

@unittest.skipIf(os.name is 'nt', 'Unable to test on Windows') # TODO windows testing refactor
def test_download_dir_dot_dir(self):
self._test_download_dir(os.path.join('.', 'a_nested_dir'))

Expand Down Expand Up @@ -553,7 +540,6 @@ def test_collection_download_self_nested(self):
self.dss.download_collection(uuid=test_col['uuid'],
replica='aws', download_dir=t)

@unittest.skipIf(os.name is 'nt', 'Unable to test on Windows') # TODO windows testing refactor
def test_collection_download_deep(self):
"""Test that we can download nested collections"""
test_cols = self._generate_col_hierarchy(4)
Expand Down

0 comments on commit 9a0f704

Please sign in to comment.