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.

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

CI: fail with warnings, and several minor tunings to test workflow #325

Merged
merged 1 commit into from
Jul 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ jobs:

- uses: actions/upload-artifact@v4
with:
name: cibw-wheels-${{ matrix.os }}-${{ strategy.job-index }}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand what's happening here fully, I would make sure to test this.

Copy link
Member Author

Choose a reason for hiding this comment

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

The -${{ strategy.job-index }} was an necessary number for the artefact name, e.g. "cibw-wheels-macos-latest-2.zip". So now it becomes just "cibw-wheels-macos-latest.zip". Artefacts can be viewed/downloaded in the GHA Summary, which I sometimes download to inspect or upload to PyPI (hopefully less-so for future releases).

Copy link
Contributor

@EwoutH EwoutH Jul 11, 2024

Choose a reason for hiding this comment

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

Doesn't this give a problem with macOS artifact names? Since cibuildwheel runs both on x86 and Arm runners?

Edit: Nevermind, they already merge them on the cibuildwheel end to a single artifact, so this doesn't change anything.

Copy link
Member Author

Choose a reason for hiding this comment

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

I just checked to see, and the files look good. Each artefact contains several wheel files created from the runner, so there's no conflict.

name: cibw-wheels-${{ matrix.os }}
path: ./wheelhouse/*.whl

build_sdist:
Expand Down
42 changes: 16 additions & 26 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,33 +4,24 @@ on:
push:
branches:
- master
paths:
- '.github/workflows/test.yml'
pull_request:
workflow_dispatch:
schedule:
- cron: '0 6 * * 1'

jobs:
pre-commit:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-python@v5
- uses: pre-commit/[email protected]

conda:
name: Conda ${{ matrix.python-version }} - ${{ matrix.os }}
defaults:
run:
shell: bash -l {0}
runs-on: ${{ matrix.os }}
strategy:
fail-fast: true
fail-fast: false
matrix:
os: ['ubuntu-latest', 'macos-latest', 'windows-latest']
python-version: ['3.9', '3.10', '3.11', '3.12']
# test oldesst and newest libspatialindex versions
# test oldest and newest libspatialindex versions
sidx-version: ['1.8.5', '2.0.0']
exclude:
- os: 'macos-latest'
Expand All @@ -43,16 +34,15 @@ jobs:
channels: conda-forge
auto-update-conda: true
python-version: ${{ matrix.python-version }}

- name: Setup
run: |
conda install -c conda-forge numpy libspatialindex=${{ matrix.sidx-version }} -y
run: conda install -c conda-forge numpy pytest libspatialindex=${{ matrix.sidx-version }} -y

- name: Install
run: |
pip install -e .
run: pip install -e .

- name: Test with pytest
run: |
pip install pytest
python -m pytest --doctest-modules rtree tests
run: pytest --import-mode=importlib -Werror -v --doctest-modules rtree tests

ubuntu:
name: Ubuntu Python ${{ matrix.python-version }}
Expand All @@ -61,7 +51,7 @@ jobs:
shell: bash -l {0}
runs-on: ubuntu-latest
strategy:
fail-fast: true
fail-fast: false
matrix:
python-version: ['3.9', '3.10', '3.11', '3.12', '3.13']

Expand All @@ -72,15 +62,15 @@ jobs:
with:
python-version: ${{ matrix.python-version }}
allow-prereleases: true

- name: Setup
run: |
sudo apt install libspatialindex-c6 python3-pip
python3 -m pip install --upgrade pip
python3 -m pip install setuptools numpy pytest
sudo apt-get -y install libspatialindex-c6
pip install --upgrade pip
pip install numpy pytest

- name: Build
run: |
python3 -m pip install --user .
run: pip install --user .

- name: Test with pytest
run: |
python3 -m pytest --doctest-modules rtree tests
run: pytest --import-mode=importlib -Werror -v --doctest-modules rtree tests
Loading