From 627dd240134fa843fc84de407e54ae619a580ec9 Mon Sep 17 00:00:00 2001 From: Alegria Aclan Date: Thu, 26 Jan 2023 09:47:10 +0000 Subject: [PATCH 1/7] Fix resuming of slice download --- pyega3/libs/data_file.py | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/pyega3/libs/data_file.py b/pyega3/libs/data_file.py index 9dde413..a58302e 100644 --- a/pyega3/libs/data_file.py +++ b/pyega3/libs/data_file.py @@ -203,17 +203,24 @@ def download_file_slice(self, file_name, start_pos, length, options=None, pbar=N self.temporary_files.add(file_name) - existing_size = os.stat(final_file_name).st_size if os.path.exists(final_file_name) else 0 - if existing_size > length: - os.remove(final_file_name) - existing_size = 0 + if os.path.exists(final_file_name): + return final_file_name + + existing_size = 0 + + if os.path.exists(file_name): + existing_size = os.stat(file_name).st_size + + if existing_size > length: + os.remove(file_name) + + if existing_size == length: + os.rename(file_name, final_file_name) + return final_file_name if pbar: pbar.update(existing_size) - if existing_size == length: - return final_file_name - try: range_start = start_pos + existing_size range_end = start_pos + length - 1 @@ -223,6 +230,7 @@ def download_file_slice(self, file_name, start_pos, length, options=None, pbar=N with self.data_client.get_stream(path, extra_headers) as r: with open(file_name, 'ba') as file_out: + self.temporary_files.add(file_name) for chunk in r.iter_content(DOWNLOAD_FILE_MEMORY_BUFFER_SIZE): file_out.write(chunk) if pbar: From 6983b10ffd867b00a0877571510bea0f24d033c8 Mon Sep 17 00:00:00 2001 From: Alegria Aclan Date: Thu, 9 Feb 2023 10:14:09 +0000 Subject: [PATCH 2/7] Add unit tests --- pyega3/libs/data_file.py | 8 ++- tests/unit/test_download_file_slice.py | 81 ++++++++++++++++++++++---- 2 files changed, 74 insertions(+), 15 deletions(-) diff --git a/pyega3/libs/data_file.py b/pyega3/libs/data_file.py index a58302e..512d496 100644 --- a/pyega3/libs/data_file.py +++ b/pyega3/libs/data_file.py @@ -204,6 +204,8 @@ def download_file_slice(self, file_name, start_pos, length, options=None, pbar=N self.temporary_files.add(file_name) if os.path.exists(final_file_name): + existing_size = os.stat(final_file_name).st_size + pbar and pbar.update(existing_size) return final_file_name existing_size = 0 @@ -213,13 +215,13 @@ def download_file_slice(self, file_name, start_pos, length, options=None, pbar=N if existing_size > length: os.remove(file_name) + existing_size = 0 if existing_size == length: os.rename(file_name, final_file_name) return final_file_name - if pbar: - pbar.update(existing_size) + pbar and pbar.update(existing_size) try: range_start = start_pos + existing_size @@ -241,7 +243,7 @@ def download_file_slice(self, file_name, start_pos, length, options=None, pbar=N if total_received != length: raise Exception(f"Slice error: received={total_received}, requested={length}, file='{file_name}'") - except Exception: + except Exception as e: if os.path.exists(file_name): os.remove(file_name) raise diff --git a/tests/unit/test_download_file_slice.py b/tests/unit/test_download_file_slice.py index 796620b..2ac6b02 100644 --- a/tests/unit/test_download_file_slice.py +++ b/tests/unit/test_download_file_slice.py @@ -124,9 +124,9 @@ def test_return_slice_file_when_existing(mock_data_server, mock_data_client, sli mock_stat.st_size = slice_file.length with patch.object(mock_data_client, 'get_stream', wraps=mock_data_client.get_stream) as get_stream_mock, \ - mock.patch("os.path.exists", lambda path: True if path == slice_file.file_name else False), \ - mock.patch("os.path.getsize", lambda path: slice_file.length), \ - mock.patch("os.stat", lambda path: mock_stat): + mock.patch("os.path.exists", lambda path: True if path == slice_file.file_name else False), \ + mock.patch("os.path.getsize", lambda path: slice_file.length), \ + mock.patch("os.stat", lambda path: mock_stat): # When: the slice file is downloaded filename = file.download_file_slice(slice_file.original_file_name, slice_file.start, slice_file.length) # Then: the existing slice file with same length is reused and data is not re-fetched @@ -134,23 +134,80 @@ def test_return_slice_file_when_existing(mock_data_server, mock_data_client, sli get_stream_mock.assert_not_called() -def test_remove_existing_slice_file_when_it_exceeds_slice_length(mock_data_server, mock_data_client, slice_file): - # Given: a slice file existing in tmp directory whose size exceeds the expected slice length +def test_remove_existing_slice_file_redownload_all_bytes_when_it_exceeds_slice_length(mock_data_server, + mock_data_client, slice_file): mock_data_server.file_content[slice_file.id] = slice_file.binary + + written_bytes = 0 + + def mock_write(buf): + nonlocal written_bytes + buf_len = len(buf) + expected_buf = slice_file.binary[slice_file.start + written_bytes:slice_file.start + written_bytes + buf_len] + assert expected_buf == buf + written_bytes += buf_len + + # for condition to meet tmp size is greater than expected - this shouldn't happen ever file = DataFile(mock_data_client, slice_file.id) + m_open = mock.mock_open() + mock_stat = Mock() mock_stat.st_size = slice_file.length + 1 - with mock.patch("os.remove") as remove_file_mock, \ - mock.patch("os.path.exists", lambda path: True if path == slice_file.file_name else False), \ - mock.patch("os.path.getsize", lambda path: slice_file.length), \ - mock.patch("os.stat", lambda path: mock_stat): - # When: the slice file is downloaded + mock_file_exists = {slice_file.file_name: False} + slice_temp_file_name = slice_file.file_name + '.tmp' + mock_file_exists[slice_temp_file_name] = True + + with mock.patch("builtins.open", m_open, create=True), \ + mock.patch("os.remove") as remove_file_mock, \ + mock.patch("os.path.exists", lambda path: mock_file_exists.get(path)), \ + mock.patch("os.path.getsize", lambda path: written_bytes if path == slice_temp_file_name else 0), \ + mock.patch("os.rename"), \ + mock.patch("os.stat", lambda path: mock_stat): + m_open().write.side_effect = mock_write output_file = file.download_file_slice(slice_file.original_file_name, slice_file.start, slice_file.length) - # Then: the existing slice file is deleted + assert slice_file.length == written_bytes assert output_file == slice_file.file_name - remove_file_mock.assert_called_once_with(slice_file.file_name) + remove_file_mock.assert_called_once_with(slice_temp_file_name) + m_open.assert_called_with(slice_temp_file_name, 'ba') + + +def test_resume_existing_tmp_slice_file(mock_data_server, mock_data_client, slice_file): + mock_data_server.file_content[slice_file.id] = slice_file.binary + + written_bytes = 0 + + # tracks bytes written, create a mock class which tracks the written bytes and another method which asserts that all expected bytes are written + def mock_write(buf): + nonlocal written_bytes + buf_len = len(buf) + written_bytes += buf_len + + # for condition to meet tmp size is greater than expected - this shouldn't happen ever + file = DataFile(mock_data_client, slice_file.id) + + m_open = mock.mock_open() + missing_bytes_length = 5 + mock_stat = Mock() + mock_stat.st_size = slice_file.length - missing_bytes_length + + mock_file_exists = {slice_file.file_name: False} + slice_temp_file_name = slice_file.file_name + '.tmp' + mock_file_exists[slice_temp_file_name] = True + + with mock.patch("builtins.open", m_open, create=True), \ + mock.patch("os.remove") as remove_file_mock, \ + mock.patch("os.path.exists", lambda path: mock_file_exists.get(path)), \ + mock.patch("os.path.getsize", lambda path: slice_file.length if path == slice_temp_file_name else 0), \ + mock.patch("os.rename"), \ + mock.patch("os.stat", lambda path: mock_stat): + m_open().write.side_effect = mock_write + output_file = file.download_file_slice(slice_file.original_file_name, slice_file.start, slice_file.length) + assert written_bytes == missing_bytes_length + assert output_file == slice_file.file_name + + m_open.assert_called_with(slice_temp_file_name, 'ba') def teardown_module(): From 484238524bd20c19f46ea8ac995919a3a876a04f Mon Sep 17 00:00:00 2001 From: Alegria Aclan Date: Thu, 9 Feb 2023 14:17:19 +0000 Subject: [PATCH 3/7] Retry whole slice to avoid md5 mismatch of whole file --- pyega3/libs/data_file.py | 16 ++------- tests/unit/test_download_file_slice.py | 47 ++------------------------ 2 files changed, 4 insertions(+), 59 deletions(-) diff --git a/pyega3/libs/data_file.py b/pyega3/libs/data_file.py index 512d496..04beca3 100644 --- a/pyega3/libs/data_file.py +++ b/pyega3/libs/data_file.py @@ -208,23 +208,11 @@ def download_file_slice(self, file_name, start_pos, length, options=None, pbar=N pbar and pbar.update(existing_size) return final_file_name - existing_size = 0 - if os.path.exists(file_name): - existing_size = os.stat(file_name).st_size - - if existing_size > length: - os.remove(file_name) - existing_size = 0 - - if existing_size == length: - os.rename(file_name, final_file_name) - return final_file_name - - pbar and pbar.update(existing_size) + os.remove(file_name) try: - range_start = start_pos + existing_size + range_start = start_pos range_end = start_pos + length - 1 extra_headers = { 'Range': f'bytes={range_start}-{range_end}' diff --git a/tests/unit/test_download_file_slice.py b/tests/unit/test_download_file_slice.py index 2ac6b02..830b1d0 100644 --- a/tests/unit/test_download_file_slice.py +++ b/tests/unit/test_download_file_slice.py @@ -134,8 +134,7 @@ def test_return_slice_file_when_existing(mock_data_server, mock_data_client, sli get_stream_mock.assert_not_called() -def test_remove_existing_slice_file_redownload_all_bytes_when_it_exceeds_slice_length(mock_data_server, - mock_data_client, slice_file): +def test_remove_existing_slice_file_redownload_slice(mock_data_server,mock_data_client, slice_file): mock_data_server.file_content[slice_file.id] = slice_file.binary written_bytes = 0 @@ -147,14 +146,10 @@ def mock_write(buf): assert expected_buf == buf written_bytes += buf_len - # for condition to meet tmp size is greater than expected - this shouldn't happen ever file = DataFile(mock_data_client, slice_file.id) m_open = mock.mock_open() - mock_stat = Mock() - mock_stat.st_size = slice_file.length + 1 - mock_file_exists = {slice_file.file_name: False} slice_temp_file_name = slice_file.file_name + '.tmp' mock_file_exists[slice_temp_file_name] = True @@ -163,8 +158,7 @@ def mock_write(buf): mock.patch("os.remove") as remove_file_mock, \ mock.patch("os.path.exists", lambda path: mock_file_exists.get(path)), \ mock.patch("os.path.getsize", lambda path: written_bytes if path == slice_temp_file_name else 0), \ - mock.patch("os.rename"), \ - mock.patch("os.stat", lambda path: mock_stat): + mock.patch("os.rename"): m_open().write.side_effect = mock_write output_file = file.download_file_slice(slice_file.original_file_name, slice_file.start, slice_file.length) assert slice_file.length == written_bytes @@ -173,43 +167,6 @@ def mock_write(buf): m_open.assert_called_with(slice_temp_file_name, 'ba') -def test_resume_existing_tmp_slice_file(mock_data_server, mock_data_client, slice_file): - mock_data_server.file_content[slice_file.id] = slice_file.binary - - written_bytes = 0 - - # tracks bytes written, create a mock class which tracks the written bytes and another method which asserts that all expected bytes are written - def mock_write(buf): - nonlocal written_bytes - buf_len = len(buf) - written_bytes += buf_len - - # for condition to meet tmp size is greater than expected - this shouldn't happen ever - file = DataFile(mock_data_client, slice_file.id) - - m_open = mock.mock_open() - missing_bytes_length = 5 - mock_stat = Mock() - mock_stat.st_size = slice_file.length - missing_bytes_length - - mock_file_exists = {slice_file.file_name: False} - slice_temp_file_name = slice_file.file_name + '.tmp' - mock_file_exists[slice_temp_file_name] = True - - with mock.patch("builtins.open", m_open, create=True), \ - mock.patch("os.remove") as remove_file_mock, \ - mock.patch("os.path.exists", lambda path: mock_file_exists.get(path)), \ - mock.patch("os.path.getsize", lambda path: slice_file.length if path == slice_temp_file_name else 0), \ - mock.patch("os.rename"), \ - mock.patch("os.stat", lambda path: mock_stat): - m_open().write.side_effect = mock_write - output_file = file.download_file_slice(slice_file.original_file_name, slice_file.start, slice_file.length) - assert written_bytes == missing_bytes_length - assert output_file == slice_file.file_name - - m_open.assert_called_with(slice_temp_file_name, 'ba') - - def teardown_module(): for f in glob.glob(f'{os.getcwd()}/*.slice'): os.remove(f) From 5b82ca9d75aab6018d5e0533ba4b0ac2c5b5c4da Mon Sep 17 00:00:00 2001 From: Alegria Aclan Date: Thu, 9 Feb 2023 15:39:10 +0000 Subject: [PATCH 4/7] Create a slice download mock and refactor unit test --- tests/unit/test_download_file_slice.py | 49 +++++++++++++------------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/tests/unit/test_download_file_slice.py b/tests/unit/test_download_file_slice.py index 830b1d0..0c8d501 100644 --- a/tests/unit/test_download_file_slice.py +++ b/tests/unit/test_download_file_slice.py @@ -28,14 +28,6 @@ def slice_file(random_binary_file): def test_download_file_slice_downloads_correct_bytes_to_file(mock_data_server, slice_file, mock_data_client): mock_data_server.file_content[slice_file.id] = slice_file.binary - written_bytes = 0 - - def mock_write(buf): - nonlocal written_bytes - buf_len = len(buf) - expected_buf = slice_file.binary[slice_file.start + written_bytes:slice_file.start + written_bytes + buf_len] - assert expected_buf == buf - written_bytes += buf_len file_name = common.rand_str() file_name_for_slice = file_name + '-from-' + str(slice_file.start) + '-len-' + str( @@ -44,12 +36,14 @@ def mock_write(buf): file = DataFile(mock_data_client, slice_file.id) m_open = mock.mock_open() + download_mock = SliceFileDownloadMock(slice_file) with mock.patch("builtins.open", m_open, create=True): - with mock.patch("os.path.getsize", lambda path: written_bytes if path == file_name_for_slice else 0): + with mock.patch("os.path.getsize", + lambda path: download_mock.written_bytes if path == file_name_for_slice else 0): with mock.patch("os.rename"): - m_open().write.side_effect = mock_write + m_open().write.side_effect = download_mock.write file.download_file_slice(file_name, slice_file.start, slice_file.length) - assert slice_file.length == written_bytes + assert slice_file.length == download_mock.written_bytes m_open.assert_called_with(file_name_for_slice, 'ba') @@ -134,18 +128,9 @@ def test_return_slice_file_when_existing(mock_data_server, mock_data_client, sli get_stream_mock.assert_not_called() -def test_remove_existing_slice_file_redownload_slice(mock_data_server,mock_data_client, slice_file): +def test_remove_existing_slice_file_and_redownload_that_slice(mock_data_server, mock_data_client, slice_file): mock_data_server.file_content[slice_file.id] = slice_file.binary - written_bytes = 0 - - def mock_write(buf): - nonlocal written_bytes - buf_len = len(buf) - expected_buf = slice_file.binary[slice_file.start + written_bytes:slice_file.start + written_bytes + buf_len] - assert expected_buf == buf - written_bytes += buf_len - file = DataFile(mock_data_client, slice_file.id) m_open = mock.mock_open() @@ -154,14 +139,16 @@ def mock_write(buf): slice_temp_file_name = slice_file.file_name + '.tmp' mock_file_exists[slice_temp_file_name] = True + slice_download_mock = SliceFileDownloadMock(slice_file) with mock.patch("builtins.open", m_open, create=True), \ mock.patch("os.remove") as remove_file_mock, \ mock.patch("os.path.exists", lambda path: mock_file_exists.get(path)), \ - mock.patch("os.path.getsize", lambda path: written_bytes if path == slice_temp_file_name else 0), \ + mock.patch("os.path.getsize", + lambda path: slice_download_mock.written_bytes if path == slice_temp_file_name else 0), \ mock.patch("os.rename"): - m_open().write.side_effect = mock_write + m_open().write.side_effect = slice_download_mock.write output_file = file.download_file_slice(slice_file.original_file_name, slice_file.start, slice_file.length) - assert slice_file.length == written_bytes + assert slice_file.length == slice_download_mock.written_bytes assert output_file == slice_file.file_name remove_file_mock.assert_called_once_with(slice_temp_file_name) m_open.assert_called_with(slice_temp_file_name, 'ba') @@ -170,3 +157,17 @@ def mock_write(buf): def teardown_module(): for f in glob.glob(f'{os.getcwd()}/*.slice'): os.remove(f) + + +class SliceFileDownloadMock: + def __init__(self, slice_file): + self.written_bytes = 0 + self.slice_file = slice_file + + def write(self, buf): + buf_len = len(buf) + start = self.slice_file.start + self.written_bytes + end = self.slice_file.start + self.written_bytes + buf_len + expected_buf = self.slice_file.binary[start:end] + assert expected_buf == buf + self.written_bytes += buf_len From b0c153947c9aa8beb9d713610b09828130b6a4fe Mon Sep 17 00:00:00 2001 From: Alegria Aclan Date: Thu, 9 Feb 2023 15:40:29 +0000 Subject: [PATCH 5/7] Rename mock to be more descriptive --- tests/unit/test_download_file_slice.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unit/test_download_file_slice.py b/tests/unit/test_download_file_slice.py index 0c8d501..34005b1 100644 --- a/tests/unit/test_download_file_slice.py +++ b/tests/unit/test_download_file_slice.py @@ -36,14 +36,14 @@ def test_download_file_slice_downloads_correct_bytes_to_file(mock_data_server, s file = DataFile(mock_data_client, slice_file.id) m_open = mock.mock_open() - download_mock = SliceFileDownloadMock(slice_file) + slice_download_mock = SliceFileDownloadMock(slice_file) with mock.patch("builtins.open", m_open, create=True): with mock.patch("os.path.getsize", - lambda path: download_mock.written_bytes if path == file_name_for_slice else 0): + lambda path: slice_download_mock.written_bytes if path == file_name_for_slice else 0): with mock.patch("os.rename"): - m_open().write.side_effect = download_mock.write + m_open().write.side_effect = slice_download_mock.write file.download_file_slice(file_name, slice_file.start, slice_file.length) - assert slice_file.length == download_mock.written_bytes + assert slice_file.length == slice_download_mock.written_bytes m_open.assert_called_with(file_name_for_slice, 'ba') From 58dd8218acef7daa93688d2eedd65ae6bc704dae Mon Sep 17 00:00:00 2001 From: Alegria Aclan Date: Thu, 9 Feb 2023 16:02:00 +0000 Subject: [PATCH 6/7] Shorten var names --- pyega3/libs/data_file.py | 3 +-- tests/unit/test_download_file_slice.py | 26 ++++++++++---------------- 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/pyega3/libs/data_file.py b/pyega3/libs/data_file.py index 04beca3..af9048f 100644 --- a/pyega3/libs/data_file.py +++ b/pyega3/libs/data_file.py @@ -223,8 +223,7 @@ def download_file_slice(self, file_name, start_pos, length, options=None, pbar=N self.temporary_files.add(file_name) for chunk in r.iter_content(DOWNLOAD_FILE_MEMORY_BUFFER_SIZE): file_out.write(chunk) - if pbar: - pbar.update(len(chunk)) + pbar and pbar.update(len(chunk)) total_received = os.path.getsize(file_name) diff --git a/tests/unit/test_download_file_slice.py b/tests/unit/test_download_file_slice.py index 34005b1..c6dc46a 100644 --- a/tests/unit/test_download_file_slice.py +++ b/tests/unit/test_download_file_slice.py @@ -28,24 +28,21 @@ def slice_file(random_binary_file): def test_download_file_slice_downloads_correct_bytes_to_file(mock_data_server, slice_file, mock_data_client): mock_data_server.file_content[slice_file.id] = slice_file.binary - file_name = common.rand_str() - file_name_for_slice = file_name + '-from-' + str(slice_file.start) + '-len-' + str( + slice_temp_file_name = file_name + '-from-' + str(slice_file.start) + '-len-' + str( slice_file.length) + '.slice.tmp' - file = DataFile(mock_data_client, slice_file.id) - m_open = mock.mock_open() - slice_download_mock = SliceFileDownloadMock(slice_file) + download_mock = SliceFileDownloadMock(slice_file) with mock.patch("builtins.open", m_open, create=True): with mock.patch("os.path.getsize", - lambda path: slice_download_mock.written_bytes if path == file_name_for_slice else 0): + lambda path: download_mock.written_bytes if path == slice_temp_file_name else 0): with mock.patch("os.rename"): - m_open().write.side_effect = slice_download_mock.write + m_open().write.side_effect = download_mock.write file.download_file_slice(file_name, slice_file.start, slice_file.length) - assert slice_file.length == slice_download_mock.written_bytes + assert slice_file.length == download_mock.written_bytes - m_open.assert_called_with(file_name_for_slice, 'ba') + m_open.assert_called_with(slice_temp_file_name, 'ba') def test_error_when_bad_token(mock_data_server, mock_data_client): @@ -130,25 +127,22 @@ def test_return_slice_file_when_existing(mock_data_server, mock_data_client, sli def test_remove_existing_slice_file_and_redownload_that_slice(mock_data_server, mock_data_client, slice_file): mock_data_server.file_content[slice_file.id] = slice_file.binary - file = DataFile(mock_data_client, slice_file.id) m_open = mock.mock_open() - mock_file_exists = {slice_file.file_name: False} slice_temp_file_name = slice_file.file_name + '.tmp' mock_file_exists[slice_temp_file_name] = True - - slice_download_mock = SliceFileDownloadMock(slice_file) + download_mock = SliceFileDownloadMock(slice_file) with mock.patch("builtins.open", m_open, create=True), \ mock.patch("os.remove") as remove_file_mock, \ mock.patch("os.path.exists", lambda path: mock_file_exists.get(path)), \ mock.patch("os.path.getsize", - lambda path: slice_download_mock.written_bytes if path == slice_temp_file_name else 0), \ + lambda path: download_mock.written_bytes if path == slice_temp_file_name else 0), \ mock.patch("os.rename"): - m_open().write.side_effect = slice_download_mock.write + m_open().write.side_effect = download_mock.write output_file = file.download_file_slice(slice_file.original_file_name, slice_file.start, slice_file.length) - assert slice_file.length == slice_download_mock.written_bytes + assert slice_file.length == download_mock.written_bytes assert output_file == slice_file.file_name remove_file_mock.assert_called_once_with(slice_temp_file_name) m_open.assert_called_with(slice_temp_file_name, 'ba') From f5259c4e1257b69c9d1f6ee5361092fe0dcb2eda Mon Sep 17 00:00:00 2001 From: Alegria Aclan Date: Thu, 9 Feb 2023 16:14:45 +0000 Subject: [PATCH 7/7] Add comments for now for clarity --- tests/unit/test_download_file_slice.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_download_file_slice.py b/tests/unit/test_download_file_slice.py index c6dc46a..6a68589 100644 --- a/tests/unit/test_download_file_slice.py +++ b/tests/unit/test_download_file_slice.py @@ -125,7 +125,8 @@ def test_return_slice_file_when_existing(mock_data_server, mock_data_client, sli get_stream_mock.assert_not_called() -def test_remove_existing_slice_file_and_redownload_that_slice(mock_data_server, mock_data_client, slice_file): +def test_remove_existing_slice_file_and_download_that_slice(mock_data_server, mock_data_client, slice_file): + # Given: a slice.tmp file existing in tmp directory mock_data_server.file_content[slice_file.id] = slice_file.binary file = DataFile(mock_data_client, slice_file.id) @@ -141,7 +142,9 @@ def test_remove_existing_slice_file_and_redownload_that_slice(mock_data_server, lambda path: download_mock.written_bytes if path == slice_temp_file_name else 0), \ mock.patch("os.rename"): m_open().write.side_effect = download_mock.write + # When: the slice file is downloaded output_file = file.download_file_slice(slice_file.original_file_name, slice_file.start, slice_file.length) + # Then: delete the existing .slice.tmp file and download that slice assert slice_file.length == download_mock.written_bytes assert output_file == slice_file.file_name remove_file_mock.assert_called_once_with(slice_temp_file_name)