Skip to content

Commit

Permalink
Avoiding a race condition when pulling device files to local, this wa…
Browse files Browse the repository at this point in the history
…s the case when initializing several emulators in parallel

PiperOrigin-RevId: 687882441
  • Loading branch information
The android_world Authors committed Oct 22, 2024
1 parent 0fa4f75 commit b1858b7
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 13 deletions.
7 changes: 2 additions & 5 deletions android_world/utils/file_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@
import datetime
import os
import random
import re
import shutil
import string
import tempfile
from typing import Iterator
from typing import Optional

Expand Down Expand Up @@ -303,16 +303,13 @@ def tmp_directory_from_device(
FileNotFoundError: If the remote directory does not exist.
RuntimeError: If there is an adb communication error.
"""
directory_name = re.sub(r"\W", "", device_path)
tmp_directory = os.path.join(TMP_LOCAL_LOCATION, directory_name)
tmp_directory = tempfile.mkdtemp()
logging.info(
"Copying %s directory to local tmp %s", device_path, tmp_directory
)

adb_utils.set_root_if_needed(env, timeout_sec)

if os.path.exists(tmp_directory):
raise FileExistsError(f"{tmp_directory} already exists.")
if not check_directory_exists(device_path, env):
raise FileNotFoundError(f"{device_path} does not exist.")
try:
Expand Down
16 changes: 8 additions & 8 deletions android_world/utils/file_utils_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,13 @@ def test_check_directory_exists(self):
@mock.patch.object(os.path, 'exists')
@mock.patch.object(file_utils, 'check_directory_exists')
@mock.patch.object(shutil, 'rmtree')
@mock.patch.object(tempfile, 'mkdtemp')
def test_tmp_directory_from_device(
self, mock_rmtree, mock_check_directory_exists, mock_path_exists
self,
mock_mkdtemp,
mock_rmtree,
mock_check_directory_exists,
mock_path_exists,
):
"""Test if tmp_directory_from_device correctly copies a directory and handles exceptions."""
mock_response = adb_pb2.AdbResponse(status=adb_pb2.AdbResponse.Status.OK)
Expand All @@ -84,7 +89,8 @@ def test_tmp_directory_from_device(
),
)

tmp_local_directory = f'{file_utils.TMP_LOCAL_LOCATION}/remotedir'
tmp_local_directory = '/tmp/random/dir'
mock_mkdtemp.return_value = tmp_local_directory
with file_utils.tmp_directory_from_device(
'/remotedir', self.mock_env
) as tmp_directory:
Expand All @@ -104,12 +110,6 @@ def test_tmp_directory_from_device(
mock_rmtree.assert_not_called()
mock_rmtree.assert_called_with(tmp_local_directory)

# Test FileExistsError
mock_path_exists.return_value = True
with self.assertRaises(FileExistsError):
with file_utils.tmp_directory_from_device('/remote/dir', self.mock_env):
pass

# Test FileNotFoundError
mock_path_exists.return_value = False
mock_check_directory_exists.return_value = False
Expand Down

0 comments on commit b1858b7

Please sign in to comment.