Repository: microsoft/vasim (language: python)

Issue:
Parallelize tests
Investigate using `pytest-xdist` to parallelize tests.


Discussion / hints:
Right now this doesn't work because multiple tests currently collide withone another's working directory when writing/deleting data from the filesystem.

This should probably be addressed through something like a `ContextManager` and `TempDirectory` that automatically cleanups up after itself.
a stupidly easy way would just to name all of the test dirs differently
Yeah, that'd work for the tests.  But I didn't look into whether this was fundamentally preventing simultaneous runs across instances for other purposes too.
________________________________
From: Karla Saur ***@***.***>
Sent: Tuesday, September 17, 2024 13:37
To: microsoft/vasim ***@***.***>
Cc: Author ***@***.***>; Comment ***@***.***>; Subscribed ***@***.***>
Subject: Re: [microsoft/vasim] Parallelize tests (Issue #60)


a stupidly easy way would just to name all of the test dirs differently

—
Reply to this email directly, view it on GitHub<https://github.com/microsoft/vasim/issues/60#issuecomment-2356636109> or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABQ53FF2TGYYKKBZWTK2NXDZXBZGPBFKMF2HI4TJMJ2XIZLTSOBKK5TBNR2WLJDUOJ2WLJDOMFWWLO3UNBZGKYLEL5YGC4TUNFRWS4DBNZ2F6YLDORUXM2LUPGBKK5TBNR2WLJDUOJ2WLJDOMFWWLLTXMF2GG2C7MFRXI2LWNF2HTAVFOZQWY5LFUVUXG43VMWSG4YLNMWVXI2DSMVQWIX3UPFYGLLDTOVRGUZLDORPXI6LQMWWES43TOVSUG33NNVSW45FGORXXA2LDOOJIFJDUPFYGLKTSMVYG643JORXXE6NFOZQWY5LFVE4DGNRUGQZDEOBQQKSHI6LQMWSWS43TOVS2K5TBNR2WLKRSGUZDSNBXHAZDSN5HORZGSZ3HMVZKMY3SMVQXIZI>.
You are receiving this email because you authored the thread.

Triage notifications on the go with GitHub Mobile for iOS<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.




Fix patch:
diff --git a/.github/workflows/pythonapp.yml b/.github/workflows/pythonapp.yml
index d81747d..01b2051 100644
--- a/.github/workflows/pythonapp.yml
+++ b/.github/workflows/pythonapp.yml
@@ -63,10 +63,10 @@ jobs:
       run: |
         python -m pip install -e .[dev]
         python -m pip install jupyter
-    - name: Coverage on basic tests
+    - name: Tests with coverage reporting
       run : |
-        coverage run -a -m pytest
-        coverage xml
+        # Run tests in parallel with coverage (configured via pyproject.toml)
+        pytest
     - name: Test notebooks
       run: |
         find . -name '*.ipynb' -print0 | xargs -0 -n1 -P0 python -m jupyter execute
diff --git a/.gitignore b/.gitignore
index 65adc81..1155638 100644
--- a/.gitignore
+++ b/.gitignore
@@ -30,6 +30,9 @@ MANIFEST
 *.pylint*
 *.vscode*
 
+!.vscode/settings.json
+!.vscode/launch.json
+
 # PyInstaller
 #  Usually these files are written by a python script from a template
 #  before PyInstaller builds the exe, so as to inject date/other infos into it.
diff --git a/.vscode/launch.json b/.vscode/launch.json
new file mode 100644
index 0000000..c1ddb20
--- /dev/null
+++ b/.vscode/launch.json
@@ -0,0 +1,18 @@
+{
+    // Use IntelliSense to learn about possible attributes.
+    // Hover to view descriptions of existing attributes.
+    // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
+    "version": "0.2.0",
+    "configurations": [
+        {
+            "name": "Debug Unit Test",
+            "type": "python",
+            "request": "test",
+            "justMyCode": false,
+            "env": {
+                // When debugging tests, only run a single pytest worker instance.
+                "PYTEST_ADDOPTS": "-n0 --no-cov --dist=no --log-level=DEBUG"
+            }
+        }
+    ]
+}
diff --git a/.vscode/settings.json b/.vscode/settings.json
new file mode 100644
index 0000000..ff8cbd0
--- /dev/null
+++ b/.vscode/settings.json
@@ -0,0 +1,8 @@
+{
+    "python.testing.pytestArgs": [
+        "--log-level=DEBUG",
+        "tests"
+    ],
+    "python.testing.unittestEnabled": false,
+    "python.testing.pytestEnabled": true
+}
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 8882d35..550a56c 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -110,11 +110,12 @@ flake8 . --count  --max-complexity=10 --max-line-length=127 --statistics
 #### Coverage
 
 For coverage, we use [coverage.py](https://coverage.readthedocs.io/en/) in our Github Actions.
-Run  `python -m pip install coverage` if you don't already have this, and any code you commit should generally not significantly impact coverage.
+Run  `python -m pip install coverage pytest-cov` if you don't already have this, and any code you commit should generally not significantly impact coverage.
+
+We strive to not let check-ins decrease coverage (as configured in [`pyproject.toml`](./pyproject.toml)).
 
-We strive to not let check-ins decrease coverage.
 To run all unit tests:
 
 ```sh
-coverage run -m pytest tests
+pytest
 ```
diff --git a/pyproject.toml b/pyproject.toml
index 857a2db..4eab76f 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -47,6 +47,8 @@ dependencies = [
     "autopep8",
     "flake8",
     "pytest",
+    "pytest-cov",
+    "pytest-xdist",
     "coverage",
     "pre-commit",
 ]
@@ -59,4 +61,10 @@ Issues = "https://github.com/microsoft/vasim/issues"
 
 # Tell setuptools where to find the package.
 [tool.setuptools]
-package-dir = {"" = "src"}
\ No newline at end of file
+package-dir = {"" = "src"}
+
+[tool.pytest.ini_options]
+minversion = "6.0"
+# parallelize tests with xdist
+# produce coverage report
+addopts = "-n auto -s -v --cov=src/ --cov-fail-under=70 --cov-report=xml"


Test patch:
diff --git a/tests/README.md b/tests/README.md
index 7d6c4d4..23b460b 100644
--- a/tests/README.md
+++ b/tests/README.md
@@ -19,4 +19,5 @@ Highlighting a few that might be relevant for those wanting to use the simulator
 
 ## Notes
 
-For our tests [data](test_data/), we use a trace from the [Alibaba dataset](https://github.com/alibaba/clusterdata).
+* For our tests [data](test_data/), we use a trace from the [Alibaba dataset](https://github.com/alibaba/clusterdata).
+* Tests use `pytest-xdist` to run in parallel by default.  To run tests serially, use `pytest -n 0`.
diff --git a/tests/test_SimulatedInfraScaler.py b/tests/test_SimulatedInfraScaler.py
index e1ac159..dffbb4f 100644
--- a/tests/test_SimulatedInfraScaler.py
+++ b/tests/test_SimulatedInfraScaler.py
@@ -32,8 +32,10 @@ def setUp(self):
         self.source_dir = root_dir / "test_data/alibaba_control_c_29247_denom_1_mini"
         # Here we'll copy the source directory to a target directory, so we can modify the target directory without
         # affecting the source directory
-        self.target_dir = root_dir / "test_data/alibaba_control_c_29247_denom_1_test_to_delete_mini"
-        self.target_dir_sim = root_dir / "test_data/alibaba_control_c_29247_denom_1_test_to_delete_mini_simulations"
+        # Use a unique directory for each worker when using xdist to parallelize tests.
+        uid = os.environ.get("PYTEST_XDIST_WORKER", "")
+        self.target_dir = root_dir / f"test_data/tmp/{uid}/alibaba_control_c_29247_denom_1_test_to_delete_mini"
+        self.target_dir_sim = root_dir / f"test_data/tmp/{uid}/alibaba_control_c_29247_denom_1_test_to_delete_mini_simulations"
         shutil.rmtree(self.target_dir, ignore_errors=True)
         shutil.copytree(self.source_dir, self.target_dir)
 
diff --git a/tests/test_config_params.py b/tests/test_config_params.py
index d0b8806..008b181 100644
--- a/tests/test_config_params.py
+++ b/tests/test_config_params.py
@@ -24,9 +24,11 @@ def setUp(self):
         self.source_dir = root_dir / "test_data/alibaba_control_c_29247_denom_1_mini"
         # Here we'll copy the source directory to a target directory, so we can modify the target directory without
         # affecting the source directory
-        self.target_dir = root_dir / "test_data/alibaba_control_c_29247_denom_1_test_to_delete_mini"
+        # Use a unique directory for each worker when using xdist to parallelize tests.
+        uid = os.environ.get("PYTEST_XDIST_WORKER", "")
+        self.target_dir = root_dir / f"test_data/tmp/{uid}/alibaba_control_c_29247_denom_1_test_to_delete_mini"
         # TODO: sometimes the output is 'simulations', sometimes it is 'tuning'. this is confusing.
-        self.target_dir_sim = root_dir / "test_data/alibaba_control_c_29247_denom_1_test_to_delete_mini_simulations"
+        self.target_dir_sim = root_dir / f"test_data/tmp/{uid}/alibaba_control_c_29247_denom_1_test_to_delete_mini_simulations"
         shutil.rmtree(self.target_dir, ignore_errors=True)
         shutil.copytree(self.source_dir, self.target_dir)
 
diff --git a/tests/test_e2e_analysis_pareto.py b/tests/test_e2e_analysis_pareto.py
index 0732dca..c6061a9 100644
--- a/tests/test_e2e_analysis_pareto.py
+++ b/tests/test_e2e_analysis_pareto.py
@@ -28,8 +28,10 @@ def setUp(self):
         self.source_dir = root_dir / "test_data/alibaba_control_c_29247_denom_1_mini"
         # Here we'll copy the source directory to a target directory, so we can modify the target directory without
         # affecting the source directory
-        self.target_dir = root_dir / "test_data/alibaba_control_c_29247_denom_1_test_to_delete_mini"
-        self.target_dir_sim = root_dir / "test_data/alibaba_control_c_29247_denom_1_test_to_delete_mini_tuning"
+        # Use a unique directory for each worker when using xdist to parallelize tests.
+        uid = os.environ.get("PYTEST_XDIST_WORKER", "")
+        self.target_dir = root_dir / f"test_data/tmp/{uid}/alibaba_control_c_29247_denom_1_test_to_delete_mini"
+        self.target_dir_sim = root_dir / f"test_data/tmp/{uid}/alibaba_control_c_29247_denom_1_test_to_delete_mini_tuning"
         shutil.rmtree(self.target_dir, ignore_errors=True)
         shutil.copytree(self.source_dir, self.target_dir)
 
diff --git a/tests/test_e2e_multi_run_tune_with_strategy.py b/tests/test_e2e_multi_run_tune_with_strategy.py
index fef273b..97db8b5 100644
--- a/tests/test_e2e_multi_run_tune_with_strategy.py
+++ b/tests/test_e2e_multi_run_tune_with_strategy.py
@@ -27,8 +27,10 @@ def setUp(self):
         self.source_dir = root_dir / "test_data/alibaba_control_c_29247_denom_1_mini"
         # Here we'll copy the source directory to a target directory, so we can modify the target directory without
         # affecting the source directory
-        self.target_dir = root_dir / "test_data/alibaba_control_c_29247_denom_1_test_to_delete_mini"
-        self.target_dir_sim = root_dir / "test_data/alibaba_control_c_29247_denom_1_test_to_delete_mini_tuning"
+        # Use a unique directory for each worker when using xdist to parallelize tests.
+        uid = os.environ.get("PYTEST_XDIST_WORKER", "")
+        self.target_dir = root_dir / f"test_data/tmp/{uid}/alibaba_control_c_29247_denom_1_test_to_delete_mini"
+        self.target_dir_sim = root_dir / f"test_data/tmp/{uid}/alibaba_control_c_29247_denom_1_test_to_delete_mini_tuning"
         shutil.rmtree(self.target_dir, ignore_errors=True)
         shutil.copytree(self.source_dir, self.target_dir)
 
diff --git a/tests/test_e2e_single_run_sim.py b/tests/test_e2e_single_run_sim.py
index 1f429bb..515eba6 100644
--- a/tests/test_e2e_single_run_sim.py
+++ b/tests/test_e2e_single_run_sim.py
@@ -25,8 +25,10 @@ def setUp(self):
         self.source_dir = root_dir / "test_data/alibaba_control_c_29247_denom_1"
         # Here we'll copy the source directory to a target directory, so we can modify the target directory without
         # affecting the source directory
-        self.target_dir = root_dir / "test_data/alibaba_control_c_29247_denom_1_test_to_delete"
-        self.target_dir_sim = root_dir / "test_data/alibaba_control_c_29247_denom_1_test_to_delete_simulations"
+        # Use a unique directory for each worker when using xdist to parallelize tests.
+        uid = os.environ.get("PYTEST_XDIST_WORKER", "")
+        self.target_dir = root_dir / f"test_data/tmp/{uid}/alibaba_control_c_29247_denom_1_test_to_delete"
+        self.target_dir_sim = root_dir / f"test_data/tmp/{uid}/alibaba_control_c_29247_denom_1_test_to_delete_simulations"
         shutil.rmtree(self.target_dir, ignore_errors=True)
         shutil.copytree(self.source_dir, self.target_dir)
 
diff --git a/tests/test_metrics_data_window.py b/tests/test_metrics_data_window.py
index a7c2ecc..5f69366 100644
--- a/tests/test_metrics_data_window.py
+++ b/tests/test_metrics_data_window.py
@@ -32,11 +32,13 @@ def setUp(self):
         self.source_dir_toosmall = root_dir / "test_data/alibaba_control_c_29247_denom_1_toosmall"
         # Here we'll copy the source directory to a target directory, so we can modify the target directory without
         # affecting the source directory
-        self.target_dir = root_dir / "test_data/alibaba_control_c_29247_denom_1_test_to_delete_mini"
-        self.target_dir_toosmall = root_dir / "test_data/alibaba_control_c_29247_denom_1_test_to_delete_toosmall"
+        # Use a unique directory for each worker when using xdist to parallelize tests.
+        uid = os.environ.get("PYTEST_XDIST_WORKER", "")
+        self.target_dir = root_dir / f"test_data/tmp/{uid}/alibaba_control_c_29247_denom_1_test_to_delete_mini"
+        self.target_dir_toosmall = root_dir / f"test_data/tmp/{uid}/alibaba_control_c_29247_denom_1_test_to_delete_toosmall"
         # TODO: sometimes the output is 'simulations', sometimes it is 'tuning'. this is confusing.
-        self.target_dir_sim = root_dir / "test_data/alibaba_control_c_29247_denom_1_test_to_delete_mini_simulations"
-        self.target_dir_sim_toosmall = root_dir / "test_data/alibaba_control_c_29247_denom_1_test_to_delete_toosmall_simulations"
+        self.target_dir_sim = root_dir / f"test_data/tmp/{uid}/alibaba_control_c_29247_denom_1_test_to_delete_mini_simulations"
+        self.target_dir_sim_toosmall = root_dir / f"test_data/tmp/{uid}/alibaba_control_c_29247_denom_1_test_to_delete_toosmall_simulations"
         shutil.rmtree(self.target_dir, ignore_errors=True)
         shutil.rmtree(self.target_dir_toosmall, ignore_errors=True)
         shutil.copytree(self.source_dir, self.target_dir)