Skip to content

Commit

Permalink
fix(autofix-fs): Make sure storage location is clear (#682)
Browse files Browse the repository at this point in the history
Local runs were getting mixed up filesystem storage dumps, we already are deleting before saving to gcs in prod but good to add a test for it too.
  • Loading branch information
jennmueng authored May 15, 2024
1 parent 5bb62e0 commit 3946552
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 0 deletions.
8 changes: 8 additions & 0 deletions src/seer/automation/codebase/storage_adapters.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,14 @@ def copy_to_workspace(self):

def save_to_storage(self):
storage_path = self.get_storage_location(self.repo_id, self.namespace_slug)

if os.path.exists(storage_path):
try:
shutil.rmtree(storage_path, ignore_errors=False)
except Exception as e:
autofix_logger.exception(e)
return False

shutil.copytree(self.tmpdir, storage_path, dirs_exist_ok=True)

return True
Expand Down
74 changes: 74 additions & 0 deletions tests/automation/codebase/test_storage_adapters.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,35 @@ def test_save_to_storage(self):
self.assertTrue(os.path.exists(storage_location))
self.assertTrue(os.path.exists(os.path.join(storage_location, "test.txt")))

@patch("seer.automation.codebase.storage_adapters.shutil.rmtree", side_effect=Exception)
@patch("seer.automation.codebase.storage_adapters.os.path.exists", return_value=True)
def test_save_to_storage_failure(self, mock_exists, mock_rmtree):
adapter = FilesystemStorageAdapter(1, "test")

os.makedirs(adapter.tmpdir, exist_ok=True)
with open(os.path.join(adapter.tmpdir, "test.txt"), "w") as f:
f.write("test")

self.assertFalse(adapter.save_to_storage())

def test_save_to_storage_overwrites_existing_files(self):
adapter = FilesystemStorageAdapter(1, "test")
storage_location = adapter.get_storage_location(1, "test")

os.makedirs(adapter.tmpdir, exist_ok=True)
with open(os.path.join(adapter.tmpdir, "test.txt"), "w") as f:
f.write("test")

storage_location = adapter.get_storage_location(1, "test")
os.makedirs(storage_location, exist_ok=True)
with open(os.path.join(storage_location, "bad.txt"), "w") as f:
f.write("bad")

self.assertTrue(adapter.save_to_storage())
self.assertTrue(os.path.exists(storage_location))
self.assertTrue(os.path.exists(os.path.join(storage_location, "test.txt")))
self.assertFalse(os.path.exists(os.path.join(storage_location, "bad.txt")))


class TestGcsStorageAdapter(unittest.TestCase):
def setUp(self) -> None:
Expand Down Expand Up @@ -148,6 +177,51 @@ def test_save_to_storage(self, mock_gcs_client):
self.assertIsNotNone(mock_blob.custom_time)
mock_blob.upload_from_filename.assert_called_with(test_file_path)

@patch("seer.automation.codebase.storage_adapters.storage.Client")
def test_save_to_storage_failure(self, mock_gcs_client):
mock_bucket = mock_gcs_client.return_value.bucket.return_value
mock_blob = MagicMock()
mock_bucket.blob.return_value = mock_blob
mock_blob.upload_from_filename.side_effect = Exception

adapter = GcsStorageAdapter(1, "test")

# Simulate files in the workspace
os.makedirs(adapter.tmpdir, exist_ok=True)
test_file_path = os.path.join(adapter.tmpdir, "test_file.txt")
with open(test_file_path, "w") as f:
f.write("This is a test file.")

self.assertFalse(adapter.save_to_storage())

@patch("seer.automation.codebase.storage_adapters.storage.Client")
def test_save_to_storage_overwrites_existing_files(self, mock_gcs_client):
mock_bucket = mock_gcs_client.return_value.bucket.return_value
mock_blob = MagicMock()
mock_bucket.blob.return_value = mock_blob
mock_blob.upload_from_filename.return_value = None

adapter = GcsStorageAdapter(1, "test")
storage_prefix = adapter.get_storage_prefix(1, "test")

# Simulate files in the workspace
os.makedirs(adapter.tmpdir, exist_ok=True)
test_file_path = os.path.join(adapter.tmpdir, "test_file.txt")
with open(test_file_path, "w") as f:
f.write("This is a test file.")

# Simulate existing files in the storage
mock_bucket.list_blobs.return_value = [mock_blob]
mock_blob.name = f"{storage_prefix}/bad_file.txt"

self.assertTrue(adapter.save_to_storage())

# Verify that the blob upload method was called with the correct path
mock_bucket.blob.assert_called_with(f"{storage_prefix}/test_file.txt")
self.assertIsNotNone(mock_blob.custom_time)
mock_blob.upload_from_filename.assert_called_with(test_file_path)
mock_blob.delete.assert_called_once()

@patch("seer.automation.codebase.storage_adapters.storage.Client")
def test_delete_from_storage(self, mock_storage_client):
mock_bucket = mock_storage_client.return_value.bucket.return_value
Expand Down

0 comments on commit 3946552

Please sign in to comment.