Fix test test_template.py::test_template (#19)

* Remove obsolete version test

Deleted the test_version.py file as it was checking for a fixed version string of "unknown," which is no longer relevant. This change helps clean up the test suite by removing unnecessary tests that do not contribute to meaningful validation.

* Add detailed assert error messages in tests

Enhanced test assertions with specific error messages to clarify differences in test outcomes. These changes improve debugging by specifying which parameters or options differ between expected and actual results. This will aid in quickly identifying and resolving issues during test failures.

* Remove redundant checklist item in PR template

The checklist item for adding code comments to explain the intent was duplicated. This change removes the redundant item to improve clarity and maintainability of the pull request template. All contributors are encouraged to check the remaining items before submitting a PR.

* Update Conda setup in GitHub Actions workflow

Replace `mamba-version` with `miniforge-version` for improved dependency management. Remove caching keys for environment and downloads, streamlining the workflow configuration. This change aims to ensure compatibility with the latest tools and simplify maintenance.

* ensure that the script is cleaned from the list of modules

Signed-off-by: Jose Borreguero <[email protected]>

.github/pull_request_template.md

@@ -23,4 +23,3 @@ updated, or an explanation is provided as to why release notes are unnecessary
     + [ ] code comments explaining the intent of code blocks
 - [ ] All the tests are passing
 - [ ] The documentation is up to date
-- [ ] code comments added when explaining intent

.github/workflows/testing.yml

@@ -16,13 +16,12 @@ jobs:
     steps:
       - uses: actions/checkout@v3
       - uses: conda-incubator/setup-miniconda@v2
+        name: Setup Conda
         with:
           auto-update-conda: true
           channels: mantid/label/main,conda-forge,defaults
-          mamba-version: "*"
+          miniforge-version: latest
           environment-file: environment.yml
-          cache-environment-key: ${{ runner.os }}-env-${{ hashFiles('**/environment.yml') }}
-          cache-downloads-key: ${{ runner.os }}-downloads-${{ hashFiles('**/environment.yml') }}
       - name: install additional dependencies
         run: |
           echo "installing additional dependencies if cannot be installed from conda"

src/mr_autoreduce/reduce_REF_M_run.py

@@ -46,8 +46,6 @@ def reduce_single_run(opts):
         os.system(f"/bin/cp {script_file} {opts['outdir']}")
         # import functions from newly created reduce_REF_M.py and reduce. Save HTML report and reduced files in outdir
         with add_to_sys_path(temp_dir):
-            if "reduce_REF_M" in sys.modules:
-                del sys.modules["reduce_REF_M"]  # need to re-import
             from reduce_REF_M import reduce_events_file, upload_html_report
 
             reports = reduce_events_file(opts["events_file"], opts["outdir"])

src/mr_reduction/simple_utils.py

@@ -7,9 +7,25 @@
 
 
 @contextmanager
-def add_to_sys_path(path):
-    r""" "Temporarily dd `path` to the PYTHONPATH"""
+def add_to_sys_path(path, clean_module_reduce_REF_M=True):
+    r"""Temporarily add `path` to the PYTHONPATH.
+
+    Parameters
+    ----------
+    path : str
+        The path to be added to the PYTHONPATH.
+    clean_module_reduce_REF_M : bool, optional
+        If True, remove the "reduce_REF_M" module from sys.modules if it exists,
+        so it can be re-imported. Default is True.
+
+    Examples
+    --------
+    with add_to_sys_path(tempdir):
+        from reduce_REF_M import reduction_user_options
+    """
     sys.path.insert(0, path)
+    if clean_module_reduce_REF_M and ("reduce_REF_M" in sys.modules):
+        del sys.modules["reduce_REF_M"]  # need to re-import
     try:
         yield
     finally:

tests/conftest.py

@@ -14,7 +14,7 @@
 this_module_path = sys.modules[__name__].__file__
 
 
-@pytest.fixture()
+@pytest.fixture()  # scope="function"
 def tempdir(tmpdir):
     r"""Get the path of pytest fixture tmpdir as a string"""
     return str(tmpdir)
@@ -73,7 +73,7 @@ def path_to(self, basename: str) -> str:
         config[key] = val
 
 
-@pytest.fixture()
+@pytest.fixture()  # scope="function"
 def mock_filesystem(tempdir, data_server):
     r"""
     A set of mocks to redirect paths such as /SNS/REF_M/%(ipts)s/shared/autoreduce/

tests/unit/mr_autoreduce/test_template.py

@@ -68,45 +68,45 @@ def test_template(data_server, tempdir):
 
         opts = reduction_user_options()
         # assert common options
-        assert opts.peak_count == common["peak_count"]
+        assert opts.peak_count == common["peak_count"], "peak_count differs"
         for a, b in [  # template and reduction keys don't have the same name
             ("plot_2d", "plot_in_2D"),
             ("const_q_binning", "use_const_q"),
             ("q_step", "q_step"),
             ("use_sangle", "use_sangle"),
             ("update_peak_range", "fit_peak_in_roi"),
         ]:
-            assert opts.common[a] == common[b]
+            assert opts.common[a] == common[b], f"{a} differs"
         # assert peak1 options
         for a, b in [
             ("force_peak_roi", "force_peak"),
             ("force_bck_roi", "force_background"),
             ("use_tight_bck", "use_side_bck"),
             ("bck_offset", "bck_width"),
         ]:
-            assert opts.peak1[a] == peak1[b]
-        assert opts.peak1["peak_roi"] == [peak1["peak_min"], peak1["peak_max"]]
-        assert opts.peak1["bck_roi"] == [peak1["bck_min"], peak1["bck_max"]]
+            assert opts.peak1[a] == peak1[b], f"{a} differs for peak 1"
+        assert opts.peak1["peak_roi"] == [peak1["peak_min"], peak1["peak_max"]], "peak_roi differs for peak 1"
+        assert opts.peak1["bck_roi"] == [peak1["bck_min"], peak1["bck_max"]], "bck_roi differs for peak 1"
         # assert peak2 options
         for a, b in [
             ("force_peak_roi", "force_peak_s2"),
             ("force_bck_roi", "force_background_s2"),
             ("use_tight_bck", "use_side_bck_s2"),
             ("bck_offset", "bck_width_s2"),
         ]:
-            assert opts.peak2[a] == peak2[b]
-        assert opts.peak2["peak_roi"] == [peak2["peak_min_s2"], peak2["peak_max_s2"]]
-        assert opts.peak2["bck_roi"] == [peak2["bck_min_s2"], peak2["bck_max_s2"]]
+            assert opts.peak2[a] == peak2[b], f"{a} differs for peak 2"
+        assert opts.peak2["peak_roi"] == [peak2["peak_min_s2"], peak2["peak_max_s2"]], "peak_roi differs for peak 2"
+        assert opts.peak2["bck_roi"] == [peak2["bck_min_s2"], peak2["bck_max_s2"]], "bck_roi differs for peak 2"
         # assert peak3 options
         for a, b in [
             ("force_peak_roi", "force_peak_s3"),
             ("force_bck_roi", "force_background_s3"),
             ("use_tight_bck", "use_side_bck_s3"),
             ("bck_offset", "bck_width_s3"),
         ]:
-            assert opts.peak3[a] == peak3[b]
-        assert opts.peak3["peak_roi"] == [peak3["peak_min_s3"], peak3["peak_max_s3"]]
-        assert opts.peak3["bck_roi"] == [peak3["bck_min_s3"], peak3["bck_max_s3"]]
+            assert opts.peak3[a] == peak3[b], f"{a} differs for peak 3"
+        assert opts.peak3["peak_roi"] == [peak3["peak_min_s3"], peak3["peak_max_s3"]], "peak_roi differs for peak 3"
+        assert opts.peak3["bck_roi"] == [peak3["bck_min_s3"], peak3["bck_max_s3"]], "bck_roi differs for peak 3"
 
 
 if __name__ == "__main__":