Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update release tests for new framework #336

Merged
merged 9 commits into from
Dec 13, 2024
Merged

Update release tests for new framework #336

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
494eade
Update release_tox.ini to reflect new directory structure
ajjackson Dec 5, 2024
2b0b417
Correct path to run_tests.py in release_tox.ini
ajjackson Dec 6, 2024
4658321
Update minimum Numpy version for release_tox.in py310-min case
ajjackson Dec 6, 2024
1557297
Try simplifying path to run_tests.py
ajjackson Dec 6, 2024
1bf3f9f
Revert "Try simplifying path to run_tests.py"
ajjackson Dec 6, 2024
43b71a3
Set up spglib, environment variables for test_release source builds
ajjackson Dec 6, 2024
42bb24e
Setup mac minrequirements file and test
ajjackson Dec 6, 2024
89c5279
Release test: platform-dependent min requirements
ajjackson Dec 6, 2024
d140a93
Revert mac spglib pin
ajjackson Dec 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .github/workflows/test_release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,40 @@ jobs:
channel-priority: true
- name: Install llvm on Macos
if: startsWith(matrix.os, 'macos')
shell: bash -l {0}
env:
# Homebrew location is different on Intel Mac
LLVM_DIR: ${{ (matrix.os == 'macos-13') && '/usr/local/opt/llvm' || '/opt/homebrew/opt/llvm' }}
run: |
brew install llvm
echo CC="${LLVM_DIR}/bin/clang" >> $GITHUB_ENV
echo LDFLAGS="-L${LLVM_DIR}/lib $LDFLAGS" >> $GITHUB_ENV
echo CPPFLAGS="-I${LLVM_DIR}/include $CPPFLAGS" >> $GITHUB_ENV

- name: Windows - find MSVC and set environment variables
if: startsWith(matrix.os, 'windows')
shell: bash -l {0}
env:
MSVC_PREFIX: 'C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC'
run: |
echo "Available MSVC installations:"
ls "$MSVC_PREFIX"

MSVC_BIN=$(ls "$MSVC_PREFIX" | tail -n 1)\\bin\\HostX64\\x64
CC="$MSVC_PREFIX\\$MSVC_BIN\\cl.exe"
echo "CC: $CC"
echo "CC=$CC" >> $GITHUB_ENV

CC_LD="$MSVC_PREFIX\\$MSVC_BIN\\link.exe"
echo "CC_LD: $CC_LD"
echo "CC_LD=$CC_LD" >> $GITHUB_ENV

- name: Update pip and install dependencies
shell: bash -l {0}
run: |
python -m pip install --upgrade pip
python -m pip install -r tests_and_analysis/ci_requirements.txt

- name: Run tests
shell: bash -l {0}
env:
Expand Down
40 changes: 31 additions & 9 deletions build_utils/release_tox.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,17 @@ skipsdist = True

[testenv]
changedir = tests_and_analysis/test
test_command = python run_tests.py --report
test_command = python {toxinidir}/../tests_and_analysis/test/run_tests.py --report
requirements_dir = {toxinidir}/../tests_and_analysis

# Test PyPI source distribution
[testenv:pypisource-{py310,py312}]
passenv = CC CC_LD LDFLAGS CPPFLAGS
install_command = python -m pip install {opts} {packages}
deps =
numpy
-r{toxinidir}/tests_and_analysis/tox_requirements.txt
spglib # Not interested here in whether source builds of spglib work
-r{[testenv]requirements_dir}/tox_requirements.txt
commands_pre =
python -m pip install \
--force-reinstall \
Expand All @@ -28,23 +31,42 @@ commands = {[testenv]test_command}
install_command = python -m pip install {opts} {packages}
deps =
numpy
-r{toxinidir}/tests_and_analysis/tox_requirements.txt
-r{[testenv]requirements_dir}/tox_requirements.txt
commands_pre =
python -m pip install \
--force-reinstall \
'euphonic[matplotlib,phonopy_reader,brille]=={env:EUPHONIC_VERSION}' \
--only-binary 'euphonic'
commands = {[testenv]test_command}

[testenv:pypi-py310-min]
[testenv:pypi-py310-min-win-linux]
install_command = python -m pip install --force-reinstall {opts} {packages}
platform =
linux: linux
windows: win32
deps =
numpy==1.21.3
numpy==1.24.0
commands_pre =
python -m pip install --force-reinstall \
-r{toxinidir}/tests_and_analysis/minimum_euphonic_requirements.txt
-r{[testenv]requirements_dir}/minimum_euphonic_requirements.txt
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The one question is whether to make this a constraints rather than a requirements file. Constraints may (should?) affect the resolver of the setup-requires in the pyproject.toml meaning you don't need to pre-install numpy.

I think though that this becomes a later problem after e.g. #332

Copy link
Collaborator Author

@ajjackson ajjackson Dec 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That does seem like a better match to what we are trying to do, yes. It still requires things to be update in two places, though. I wonder if the cleanest option is to replace the pip install with uv pip install --resolution=lowest and automatically detect the lowest supported version.

It might be worth bumping the minimum supported h5py to get rid of the platform-dependent complication...

python -m pip install --force-reinstall \
-r{toxinidir}/tests_and_analysis/tox_requirements.txt
-r{[testenv]requirements_dir}/tox_requirements.txt
python -m pip install \
'euphonic[matplotlib,phonopy_reader,brille]=={env:EUPHONIC_VERSION}' \
--only-binary 'euphonic'
commands = {[testenv]test_command}

[testenv:pypi-py310-min-mac]
install_command = python -m pip install --force-reinstall {opts} {packages}
platform =
mac: darwin
deps =
numpy==1.24.0
commands_pre =
python -m pip install --force-reinstall \
-r{[testenv]requirements_dir}/minimum_euphonic_requirements_mac.txt
python -m pip install --force-reinstall \
-r{[testenv]requirements_dir}/tox_requirements.txt
python -m pip install \
'euphonic[matplotlib,phonopy_reader,brille]=={env:EUPHONIC_VERSION}' \
--only-binary 'euphonic'
Expand All @@ -57,7 +79,7 @@ conda_channels =
conda-forge
default
conda_deps =
--file={toxinidir}/tests_and_analysis/tox_requirements.txt
--file={[testenv]requirements_dir}/tox_requirements.txt
commands_pre =
conda install -c conda-forge euphonic={env:EUPHONIC_VERSION} matplotlib-base pyyaml h5py
# Brille not available on conda
Expand All @@ -72,7 +94,7 @@ conda_channels =
conda-forge
default
conda_deps =
--file={toxinidir}/tests_and_analysis/tox_requirements.txt
--file={[testenv]requirements_dir}/tox_requirements.txt
commands_pre =
conda install numpy=1.22
conda install -c conda-forge euphonic={env:EUPHONIC_VERSION} matplotlib-base pyyaml h5py
Expand Down
10 changes: 10 additions & 0 deletions tests_and_analysis/minimum_euphonic_requirements_mac.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
numpy==1.24.0
scipy==1.10.0
spglib==1.9.4.2
seekpath==1.1.0
pint==0.22
matplotlib==3.8
h5py==3.7
PyYAML==6.0
threadpoolctl==3.0.0
toolz==0.12.1
21 changes: 20 additions & 1 deletion tox.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[tox]
# The python environments to run the tests in
envlist = py310,py311,py312,py310-{base,matplotlib,phonopy_reader,brille,all},py310-minrequirements-linux
envlist = py310,py311,py312,py310-{base,matplotlib,phonopy_reader,brille,all},py310-minrequirements-{mac,linux}
# Skip the execution of setup.py as we do it with the correct arg in commands_pre below
skipsdist = True
whitelist_externals = git
Expand Down Expand Up @@ -103,3 +103,22 @@ commands_pre =
rm -rf {toxinidir}/build
python -m pip install '{toxinidir}[matplotlib,phonopy_reader,brille]'
commands = {[testenv]test_command}

[testenv:py310-minrequirements-mac]
whitelist_externals = rm
install_command =
python -m pip install --force-reinstall {opts} {packages}
platform =
macos: darwin
deps =
numpy==1.24.0
{[testenv:py310]deps}
commands_pre =
python -m pip install --force-reinstall \
-r{toxinidir}/tests_and_analysis/minimum_euphonic_requirements_mac.txt
python -m pip install --force-reinstall \
-r{toxinidir}/tests_and_analysis/tox_requirements.txt
# Force rebuild of euphonic extension to avoid Numpy clash
rm -rf {toxinidir}/build
python -m pip install '{toxinidir}[matplotlib,phonopy_reader,brille]'
commands = {[testenv]test_command}
Loading