From b3c5ebe5780cd9b649a609aff62a8fabf97efaee Mon Sep 17 00:00:00 2001 From: Jose Borreguero Date: Thu, 5 Dec 2024 10:54:04 -0500 Subject: [PATCH] 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 --- .github/pull_request_template.md | 1 - .github/workflows/testing.yml | 5 ++--- src/mr_autoreduce/reduce_REF_M_run.py | 2 -- src/mr_reduction/simple_utils.py | 20 ++++++++++++++++++-- tests/conftest.py | 4 ++-- tests/test_version.py | 5 ----- tests/unit/mr_autoreduce/test_template.py | 22 +++++++++++----------- 7 files changed, 33 insertions(+), 26 deletions(-) delete mode 100644 tests/test_version.py diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index d0531d7..a0b3eb9 100644 --- a/.github/pull_request_template.md +++ b/.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 diff --git a/.github/workflows/testing.yml b/.github/workflows/testing.yml index 9ac948a..afe2795 100644 --- a/.github/workflows/testing.yml +++ b/.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" diff --git a/src/mr_autoreduce/reduce_REF_M_run.py b/src/mr_autoreduce/reduce_REF_M_run.py index 590aafd..b3a62c0 100755 --- a/src/mr_autoreduce/reduce_REF_M_run.py +++ b/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"]) diff --git a/src/mr_reduction/simple_utils.py b/src/mr_reduction/simple_utils.py index 5bbdb61..3facab6 100644 --- a/src/mr_reduction/simple_utils.py +++ b/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: diff --git a/tests/conftest.py b/tests/conftest.py index 9b86152..5e5c09d 100644 --- a/tests/conftest.py +++ b/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/ diff --git a/tests/test_version.py b/tests/test_version.py deleted file mode 100644 index 74d3503..0000000 --- a/tests/test_version.py +++ /dev/null @@ -1,5 +0,0 @@ -from mr_reduction import __version__ - - -def test_version(): - assert __version__ == "unknown" diff --git a/tests/unit/mr_autoreduce/test_template.py b/tests/unit/mr_autoreduce/test_template.py index d2c1fa5..575f246 100644 --- a/tests/unit/mr_autoreduce/test_template.py +++ b/tests/unit/mr_autoreduce/test_template.py @@ -68,7 +68,7 @@ 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"), @@ -76,7 +76,7 @@ def test_template(data_server, tempdir): ("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"), @@ -84,9 +84,9 @@ def test_template(data_server, tempdir): ("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"), @@ -94,9 +94,9 @@ def test_template(data_server, tempdir): ("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"), @@ -104,9 +104,9 @@ def test_template(data_server, tempdir): ("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__":