From 1f7f21e83103be8ced103e219406eee99963dee1 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Fri, 13 Nov 2026 11:07:33 -0500 Subject: [PATCH] test: replace the _tests profile with a unit test of profile scripts The _tests profile downloaded pinned release tarballs of srreal, srfit, pyobjcryst, structure and utils and ran their test suites against the conda-forge versions. It broke whenever those drifted apart (srfit 3.3.0 tests need six, which is no longer installed) and the URLs had to be updated by hand. Remove the profile and its scripts, install the pdf pack in CI instead, and cover script extras with a test in tests/test_profilesmanager.py. Co-Authored-By: Claude Opus 5.5 --- .../matrix-and-codecov-on-merge-to-main.yml | 4 +- .github/workflows/tests-on-pr.yml | 4 +- docs/source/available-profiles.rst | 10 -- news/replace-tests-profile.rst | 23 ++++ requirements/packs/scripts/_pytest.bat | 119 ------------------ requirements/packs/scripts/_pytest.sh | 64 ---------- requirements/packs/scripts/tar_url.txt | 5 - requirements/profiles/_tests.yml | 11 -- tests/test_profilesmanager.py | 51 ++++++++ 9 files changed, 78 insertions(+), 213 deletions(-) create mode 100644 news/replace-tests-profile.rst delete mode 100644 requirements/packs/scripts/_pytest.bat delete mode 100644 requirements/packs/scripts/_pytest.sh delete mode 100644 requirements/packs/scripts/tar_url.txt delete mode 100644 requirements/profiles/_tests.yml diff --git a/.github/workflows/matrix-and-codecov-on-merge-to-main.yml b/.github/workflows/matrix-and-codecov-on-merge-to-main.yml index 7cb69d5..4724e62 100644 --- a/.github/workflows/matrix-and-codecov-on-merge-to-main.yml +++ b/.github/workflows/matrix-and-codecov-on-merge-to-main.yml @@ -32,7 +32,7 @@ jobs: conda list | grep -i ipympl fi - echo "Running tests for diffpy.cmi dependencies" - cmi install _tests + echo "Installing the pdf pack" + cmi install pdf secrets: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} diff --git a/.github/workflows/tests-on-pr.yml b/.github/workflows/tests-on-pr.yml index 4ac6831..8ced0cf 100644 --- a/.github/workflows/tests-on-pr.yml +++ b/.github/workflows/tests-on-pr.yml @@ -29,7 +29,7 @@ jobs: conda list | grep -i ipympl fi - echo "Running tests for diffpy.cmi dependencies" - cmi install _tests + echo "Installing the pdf pack" + cmi install pdf secrets: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} diff --git a/docs/source/available-profiles.rst b/docs/source/available-profiles.rst index 081c0a4..4759959 100644 --- a/docs/source/available-profiles.rst +++ b/docs/source/available-profiles.rst @@ -14,13 +14,3 @@ This pack was created by members of the Billinge Group and the DiffPy community. packs: ``core``, ``pdf``, ``plotting`` extras: ``ipykernel`` - -====== -_tests -====== - -Testing and development tools for ``diffpy.cmi``. - -packs: ``core``, ``pdf``, ``tests`` - -extras: ``_pytest.sh tar_url.txt``, ``_pytest.bat tar_url.txt`` diff --git a/news/replace-tests-profile.rst b/news/replace-tests-profile.rst new file mode 100644 index 0000000..9dbe8ee --- /dev/null +++ b/news/replace-tests-profile.rst @@ -0,0 +1,23 @@ +**Added:** + +* Add a unit test that checks a profile's extras scripts run with their arguments when the profile is installed. + +**Changed:** + +* Install the ``pdf`` pack in CI instead of the ``_tests`` profile. + +**Deprecated:** + +* + +**Removed:** + +* Remove the ``_tests`` profile and its ``_pytest.sh``, ``_pytest.bat`` and ``tar_url.txt`` scripts, which ran the test suites of pinned releases of ``diffpy.cmi``'s dependencies. + +**Fixed:** + +* + +**Security:** + +* diff --git a/requirements/packs/scripts/_pytest.bat b/requirements/packs/scripts/_pytest.bat deleted file mode 100644 index b3de7f1..0000000 --- a/requirements/packs/scripts/_pytest.bat +++ /dev/null @@ -1,119 +0,0 @@ -@echo off -setlocal EnableExtensions EnableDelayedExpansion - -REM ---- temp workspace ---- -set "TMPROOT=%TEMP%\remote_tests_%RANDOM%%RANDOM%" -md "%TMPROOT%" || (echo Failed to create TMPROOT & exit /b 1) - -REM ---- resolve args -> URL list file ---- -if "%~2"=="" ( - if exist "%~1" ( - set "URLS_FILE=%~f1" - ) else ( - set "URLS_FILE=%TMPROOT%\urls.txt" - > "%URLS_FILE%" echo %~1 - ) -) else ( - set "URLS_FILE=%TMPROOT%\urls.txt" - (for %%U in (%*) do @echo %%~U) > "%URLS_FILE%" -) - -pushd "%TMPROOT%" >nul || (echo Failed to enter TMPROOT & exit /b 1) - -set /a i=0 -set /a overall_ec=0 - -for /f "usebackq delims=" %%L in ("%URLS_FILE%") do call :process_one "%%L" - -popd >nul - -REM -------- CLEANUP: do not poison exit code -------- -rmdir /s /q "%TMPROOT%" >nul 2>&1 -ver >nul REM reset errorlevel to 0 - -REM ---- FINAL EXIT ---- -exit /b %overall_ec% - - -REM =================================================== -:process_one -setlocal EnableExtensions EnableDelayedExpansion - -set "url=%~1" -if "%url%"=="" (endlocal & goto :eof) - -:trim -if not "%url:~0,1%"==" " goto :trim_done -set "url=%url:~1%" -goto trim -:trim_done - -if "%url:~0,1%"=="#" (endlocal & goto :eof) - -endlocal & set /a i+=1 & set "URL=%url%" - -echo( -echo ==> [%i%] %URL% - -set "PKGDIR=%TMPROOT%\pkg_%i%" -md "%PKGDIR%" >nul 2>&1 -pushd "%PKGDIR%" >nul || goto :after - -curl -L --fail -o "archive.tar.gz" "%URL%" -if errorlevel 1 ( - echo curl failed - set /a overall_ec=1 - popd >nul & goto :after -) - -tar -xzf "archive.tar.gz" >nul 2>&1 -if errorlevel 1 tar -xf "archive.tar.gz" >nul 2>&1 -if errorlevel 1 ( - echo tar extract failed - set /a overall_ec=1 - popd >nul & goto :after -) - -set "FIRST=" -for /f "delims=" %%F in ('tar -tzf "archive.tar.gz" 2^>nul') do set "FIRST=%%F" & goto got_first -for /f "delims=" %%F in ('tar -tf "archive.tar.gz" 2^>nul') do set "FIRST=%%F" & goto got_first -:got_first - -set "PROJROOT=%CD%" -if defined FIRST for /f "tokens=1 delims=/" %%T in ("%FIRST%") do if exist ".\%%T\" set "PROJROOT=%CD%\%%T" - -if exist "%PROJROOT%\src\" rmdir /s /q "%PROJROOT%\src" >nul 2>&1 & ver >nul - -pushd "%PROJROOT%" >nul -echo Running pytest in "%CD%" - -set "OLD_PYTHONPATH=%PYTHONPATH%" - -if exist "tests\" ( - if defined OLD_PYTHONPATH ( - set "PYTHONPATH=%CD%;tests;%OLD_PYTHONPATH%" - ) else ( - set "PYTHONPATH=%CD%;tests" - ) -) else ( - if defined OLD_PYTHONPATH ( - set "PYTHONPATH=%CD%;%OLD_PYTHONPATH%" - ) else ( - set "PYTHONPATH=%CD%" - ) -) - -pytest -if errorlevel 1 set /a overall_ec=1 - -set "PYTHONPATH=%OLD_PYTHONPATH%" -popd >nul - -popd >nul - -:after -REM cleanup without poisoning errorlevel -rmdir /s /q "%PKGDIR%" >nul 2>&1 -ver >nul - -goto :eof diff --git a/requirements/packs/scripts/_pytest.sh b/requirements/packs/scripts/_pytest.sh deleted file mode 100644 index 887797f..0000000 --- a/requirements/packs/scripts/_pytest.sh +++ /dev/null @@ -1,64 +0,0 @@ -#!/usr/bin/env bash -# Usage: -# ./_pytest.sh urls.txt -# ./_pytest.sh https://host/a.tar.gz https://host/b.tgz -# From ChatGPT -set -euo pipefail - -URLS=() -if (($# == 1)) && [ -f "$1" ]; then - URLS_FILE="$1" - case "$URLS_FILE" in /*) ;; *) URLS_FILE="$PWD/$URLS_FILE" ;; esac - while IFS= read -r line; do - [[ -z "${line// }" || "$line" =~ ^[[:space:]]*# ]] && continue - URLS+=("$line") - done < "$URLS_FILE" -else - URLS=("$@") -fi - -START_DIR="$PWD" -TMPROOT="$(TMPDIR="$START_DIR" mktemp -d -t .tmp_remote_tests.XXXXXXXX)" -trap 'cd "$START_DIR" 2>/dev/null || true; rm -rf -- "$TMPROOT" || true' EXIT -cd "$TMPROOT" - -overall_ec=0 -i=0 - -for url in "${URLS[@]}"; do - ((++i)) - printf '\n==> [%d] %s\n' "$i" "$url" - - tfile="$(TMPDIR="$TMPROOT" mktemp -t "dl_${i}.XXXXXXXX")" - tarball="${tfile}.tar.gz" - curl -L --fail -o "$tarball" "$url" - - pkgdir="$TMPROOT/pkg_${i}" - mkdir -p "$pkgdir" - tar -xzf "$tarball" -C "$pkgdir" 2>/dev/null || tar -xf "$tarball" -C "$pkgdir" - - first_entry="$(tar -tzf "$tarball" 2>/dev/null | head -1 || true)" - if [ -z "$first_entry" ]; then - first_entry="$(tar -tf "$tarball" 2>/dev/null | head -1 || true)" - fi - - top="${first_entry%%/*}" - if [ -n "$top" ] && [ -d "$pkgdir/$top" ]; then - projroot="$pkgdir/$top" - else - projroot="$pkgdir" - fi - - [ -d "$projroot/src" ] && rm -rf -- "$projroot/src" || true - - if [ -d "$projroot/tests" ]; then - ( cd "$projroot" && PYTHONPATH="$PWD:tests:${PYTHONPATH:-}" pytest ) || overall_ec=1 - else - ( cd "$projroot" && PYTHONPATH="$PWD:${PYTHONPATH:-}" pytest ) || overall_ec=1 - fi - - rm -f -- "$tarball" "$tfile" || true - rm -rf -- "$pkgdir" || true -done - -exit "$overall_ec" diff --git a/requirements/packs/scripts/tar_url.txt b/requirements/packs/scripts/tar_url.txt deleted file mode 100644 index 35d298f..0000000 --- a/requirements/packs/scripts/tar_url.txt +++ /dev/null @@ -1,5 +0,0 @@ -https://github.com/diffpy/diffpy.srreal/archive/refs/tags/1.4.0.tar.gz -https://github.com/diffpy/diffpy.srfit/archive/refs/tags/3.3.0.tar.gz -https://github.com/diffpy/pyobjcryst/archive/refs/tags/2025.1.0.tar.gz -https://github.com/diffpy/diffpy.structure/archive/refs/tags/3.4.0.tar.gz -https://github.com/diffpy/diffpy.utils/archive/refs/tags/3.7.2.tar.gz diff --git a/requirements/profiles/_tests.yml b/requirements/profiles/_tests.yml deleted file mode 100644 index b48aaf8..0000000 --- a/requirements/profiles/_tests.yml +++ /dev/null @@ -1,11 +0,0 @@ -# script name must coincide with conda installation name -# script named "_*" can escape install presence check - -packs: - - core - - pdf - - tests - -extras: - - _pytest.sh tar_url.txt - - _pytest.bat tar_url.txt diff --git a/tests/test_profilesmanager.py b/tests/test_profilesmanager.py index a9656c5..7f400fa 100644 --- a/tests/test_profilesmanager.py +++ b/tests/test_profilesmanager.py @@ -45,3 +45,54 @@ def fake_check_profile(identifier): captured = capsys.readouterr() actual = captured.out assert actual.strip() == expected.strip() + + +@pytest.mark.parametrize( + "script_exit_code, expected_install_exit_code", + [ + # Test that installing a profile runs the scripts listed in its + # extras with their arguments and reports whether they succeeded + # C1: A user installs a profile whose script runs successfully. + # Expected: The script runs with its argument and the + # installation returns exit code 0. + (0, 0), + # C2: A user installs a profile whose script fails. + # Expected: The script runs with its argument and the + # installation returns exit code 1. + (1, 1), + ], +) +def test_install_with_script_extras( + script_exit_code, expected_install_exit_code, tmp_path +): + # Write a profile that lists the same script for Linux/macOS and Windows + profiles_dir = tmp_path / "requirements" / "profiles" + profiles_dir.mkdir(parents=True) + (profiles_dir / "scripts-profile.yml").write_text( + "packs: []\n" + "\n" + "extras:\n" + " - _write_message.sh hello\n" + " - _write_message.bat hello\n" + ) + # Write scripts that save their argument to a file and exit with + # script_exit_code + scripts_dir = tmp_path / "requirements" / "packs" / "scripts" + scripts_dir.mkdir(parents=True) + (scripts_dir / "_write_message.sh").write_text( + 'echo "$1" > script-output.txt\n' f"exit {script_exit_code}\n" + ) + (scripts_dir / "_write_message.bat").write_text( + "@echo off\n" + "echo %1> script-output.txt\n" + f"exit /b {script_exit_code}\n" + ) + # Install the profile + pfm = ProfilesManager(root_path=tmp_path) + actual_install_exit_code = pfm.install("scripts-profile") + actual_script_output = ( + (scripts_dir / "script-output.txt").read_text().strip() + ) + expected_script_output = "hello" + assert actual_install_exit_code == expected_install_exit_code + assert actual_script_output == expected_script_output