Merge pull request #387 from aiondemand/fix/flaky-test

* Rename file variables and use them throughout

* Use tmp_path fixture over TmpDir

* Use Pathlib more

* Extract the setup part of the test to a fixture

* Add documentation

* Repeat flaky test a number of times for testing on CI/CD

* Add sleep to see if that fixes the issue

* Reduce sleep time to see if it is still sufficient to avoid flaky

* Remove extra parametrization that was used for debugging

* Move sleep based on feedback

* Remove parameter again

* Add back sleep for CI

src/tests/backups/test_backup_restore.py

@@ -4,6 +4,9 @@
 """
 
 import os
+import time
+from typing import NamedTuple, Callable, Iterator, Protocol
+
 import pytest
 import subprocess
 
@@ -44,6 +47,66 @@ def call_restore_command(command: list[str], env: dict) -> subprocess.CompletedP
     return subprocess.run(command, env=env, input="y\n", text=True, check=True)
 
 
+class RestoreFunc(Protocol):
+    def __call__(self, level: int | None = None) -> subprocess.CompletedProcess:
+        ...
+
+
+class DataEnvironment(NamedTuple):
+    backup: Callable[[], subprocess.CompletedProcess]
+    restore: RestoreFunc
+    file1: Path
+    file2: Path
+
+
+@pytest.fixture
+def data_environment(tmp_path) -> Iterator[DataEnvironment]:
+    """Sets up a temporary directory with files, and provides backup/restore commands.
+
+    The directory structure created looks like:
+      - temporary directory/
+        - data/
+          - original/
+            - file1.txt (content: "Content of file1.txt\n")
+            - file2.txt (content: "Content of file2.txt\n")
+        - backups/
+
+    The files are accessible as `file1` and `file2`.
+    The `backup()` function calls the backup script with no arguments and environment variables set.
+    The `restore(level)` function calls the restore script, optionally for a specific level,
+      with environment variables set.
+    """
+    cycle_length = 2
+    data_name = "original"
+
+    backup_command = ["bash", str(SCRIPTS_PATH / "backup.sh"), data_name, str(cycle_length)]
+    restore_command = ["bash", str(SCRIPTS_PATH / "restore.sh"), data_name, "0"]
+
+    file_directory = tmp_path / "data" / data_name
+    file_directory.mkdir(parents=True)
+    backup_dir = tmp_path / "backups"
+    backup_dir.mkdir(parents=True)
+
+    env = os.environ.copy()
+    env["ENV_MODE"] = "testing"
+    env["DATA_PATH"] = str(tmp_path / "data")
+    env["BACKUP_PATH"] = str(backup_dir)
+
+    def backup() -> subprocess.CompletedProcess:
+        return call_backup_command(backup_command, env)
+
+    def restore(level: int | None = None) -> subprocess.CompletedProcess:
+        level_args = ["--level", str(level)] if level is not None else []
+        return call_restore_command(restore_command + level_args, env)
+
+    file1 = file_directory / "file1.txt"
+    file1.write_text("Content of file1.txt\n")
+    file2 = file_directory / "file2.txt"
+    file2.write_text("Content of file2.txt\n")
+
+    yield DataEnvironment(backup, restore, file1, file2)
+
+
 def test_backup_happy_path():
     """
     Test if the backup.sh script generates the correct directory structure.
@@ -86,50 +149,34 @@ def test_backup_happy_path():
             assert backup_file.exists()
 
 
-def test_restore_happy_path():
+def test_restore_happy_path(data_environment: DataEnvironment):
     """
     Test if the restore.sh script restores the files properly.
     Usage:
     `bash restore.sh <data to restore:str> <cycle label:int> [--level|-l <level:int>]`
     """
-    with TemporaryDirectory() as tmpdir:
-        cycle_length = 2
-        data_name = "original"
+    backup, restore, file1, file2 = data_environment
 
-        data_dir = tmpdir / Path("data")
-        original_dir = tmpdir / Path("data") / data_name
-        backup_dir = tmpdir / Path("backups")
-        env = os.environ.copy()
-        env["ENV_MODE"] = "testing"
-        env["DATA_PATH"] = str(data_dir)
-        env["BACKUP_PATH"] = str(backup_dir)
+    assert file1.read_text() == "Content of file1.txt\n"
+    assert file2.read_text() == "Content of file2.txt\n"
 
-        os.makedirs(original_dir)
-        os.makedirs(backup_dir)
-        create_test_files(original_dir, ["file1.txt", "file2.txt"])
+    backup()
+    file1.unlink()
+    file2.write_text("Content of new file\n")
+    time.sleep(0.1)  # See #387
 
-        restored_file1_path = original_dir / "file1.txt"
-        restored_file2_path = original_dir / "file2.txt"
+    backup()
+    microsecond = 10**-6
+    time.sleep(microsecond)  # See #387
+    restore()
 
-        backup_command = ["bash", SCRIPTS_PATH / "backup.sh", data_name, str(cycle_length)]
-        restore_command = ["bash", SCRIPTS_PATH / "restore.sh", data_name, "0"]
-
-        call_backup_command(backup_command, env)
-        os.remove(original_dir / "file1.txt")
-        with open(original_dir / "file2.txt", "w") as f:
-            f.write("Content of new file\n")
-        call_backup_command(backup_command, env)
-
-        call_restore_command(restore_command, env)
-        assert not restored_file1_path.exists(), f"{restored_file1_path} shouldn't be here."
-        assert restored_file2_path.exists(), f"{restored_file2_path} should be here."
-        with open(restored_file2_path, "r") as f:
-            file_content = f.read()
-        assert "Content of new file\n" in file_content
-
-        call_restore_command(restore_command + ["--level", "0"], env)
-        assert restored_file1_path.exists(), f"{restored_file1_path} should be here."
-        assert restored_file2_path.exists(), f"{restored_file2_path} should be here."
-
-        call_restore_command(restore_command + ["-l", "1"], env)
-        assert not restored_file1_path.exists(), f"{restored_file1_path} shouldn't be here."
+    assert not file1.exists(), f"{file1} shouldn't be here."
+    assert file2.exists(), f"{file2} should be here."
+    assert "Content of new file\n" in file2.read_text()
+
+    restore(level=0)
+    assert file1.exists(), f"{file1} should be here."
+    assert file2.exists(), f"{file2} should be here."
+
+    restore(level=1)
+    assert not file1.exists(), f"{file1} shouldn't be here."