-
Notifications
You must be signed in to change notification settings - Fork 129
Only run test job after basic checks are green to reduce wasted CI work. #249
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
Changes from 3 commits
6696698
9d33e3b
8b7ce5c
18e6863
660ba20
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,8 @@ jobs: | |
| toolchain: stable | ||
| profile: minimal | ||
| components: rustfmt, clippy | ||
| - uses: Swatinem/rust-cache@v1 | ||
| continue-on-error: true | ||
| - env: | ||
| CLIPPYFLAGS: --deny warnings --allow clippy::needless-lifetimes | ||
| run: | | ||
|
|
@@ -27,49 +29,60 @@ jobs: | |
| test: | ||
| name: python${{ matrix.python-version }}-${{ matrix.platform.python-architecture }} ${{ matrix.platform.os }} | ||
| runs-on: ${{ matrix.platform.os }} | ||
| needs: [lint, check-msrv, linalg-example] | ||
| strategy: | ||
| max-parallel: 16 | ||
| fail-fast: false | ||
| matrix: | ||
| python-version: [3.6, 3.7, 3.8, 3.9] | ||
| python-version: [3.7, 3.8, 3.9] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding 3.10, and 3.11-dev (if numpy supports them?)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did, but it seems that this does not work without a Git dependency on PyO3 until 0.16 is released. |
||
| platform: [ | ||
| { os: "ubuntu-latest", python-architecture: "x64", rust-target: "x86_64-unknown-linux-gnu" }, | ||
| { os: "macOS-latest", python-architecture: "x64", rust-target: "x86_64-apple-darwin" }, | ||
| { os: "windows-latest", python-architecture: "x64", rust-target: "x86_64-pc-windows-msvc" }, | ||
| { os: "windows-latest", python-architecture: "x86", rust-target: "i686-pc-windows-msvc" }, | ||
| ] | ||
| include: | ||
| # PyPy and NumPy on macOS and Windows is too slow and brittle | ||
| - python-version: pypy-3.7 | ||
| platform: { os: "ubuntu-latest", python-architecture: "x64", rust-target: "x86_64-unknown-linux-gnu" } | ||
| - python-version: pypy-3.8 | ||
| platform: { os: "ubuntu-latest", python-architecture: "x64", rust-target: "x86_64-unknown-linux-gnu" } | ||
| steps: | ||
| - uses: actions/checkout@v2 | ||
| - name: Set up Python ${{ matrix.python-version }} | ||
| uses: actions/setup-python@v2 | ||
| with: | ||
| python-version: ${{ matrix.python-version }} | ||
| architecture: ${{ matrix.platform.python-architecture }} | ||
| - name: Install Rust | ||
| uses: actions-rs/toolchain@v1 | ||
| with: | ||
| toolchain: stable | ||
| profile: minimal | ||
| target: ${{ matrix.platform.rust-target }} | ||
| default: true | ||
| - run: rustup set default-host ${{ matrix.platform.rust-target }} | ||
| - name: Enable Cargo resolver v2 to avoid PyO3 features missing in PyPy | ||
| run: echo 'resolver = "2"' >> Cargo.toml | ||
| - name: Build without default features | ||
| run: cargo build --no-default-features --verbose | ||
| run: cargo build --no-default-features | ||
| - name: Build with default features | ||
| run: cargo build --verbose | ||
| - name: Install test dependencies | ||
| run: | | ||
| python -m pip install --upgrade pip | ||
| pip install maturin numpy poetry | ||
| run: cargo build | ||
| - name: Run cargo test | ||
| run: cargo test --verbose | ||
| - name: Test Examples | ||
| run: | | ||
| for example_dir in 'examples/simple-extension'; do | ||
| pushd $example_dir && \ | ||
| poetry install && \ | ||
| poetry run maturin develop && \ | ||
| poetry run pytest && \ | ||
| popd | ||
| done | ||
| shell: bash | ||
| pip install numpy | ||
| cargo test | ||
| # Not on PyPy, because no embedding API | ||
| if: ${{ !startsWith(matrix.python-version, 'pypy') }} | ||
| - name: Install poetry | ||
| run: pip install poetry | ||
| - name: Test example | ||
| run: | | ||
| poetry install | ||
| poetry run maturin develop | ||
| poetry run pytest | ||
| working-directory: examples/simple-extension | ||
| env: | ||
| CARGO_TERM_VERBOSE: true | ||
| CARGO_BUILD_TARGET: ${{ matrix.platform.rust-target }} | ||
| RUST_BACKTRACE: 1 | ||
|
|
||
| check-msrv: | ||
|
|
@@ -86,31 +99,34 @@ jobs: | |
| profile: minimal | ||
| toolchain: 1.48.0 | ||
| default: true | ||
| - name: Install maturin, poetry, and toml | ||
| run: pip install maturin poetry toml | ||
| - name: Create an isolated example directory | ||
| run: cp -r examples/simple-extension/ ../simple-extension-msrv | ||
| - name: Edit Cargo.toml and change the path of rust-numpy | ||
| - uses: Swatinem/rust-cache@v1 | ||
| with: | ||
| working-directory: examples/simple-extension | ||
| continue-on-error: true | ||
| - name: Install toml and poetry | ||
| run: pip install toml poetry | ||
| - name: Edit Cargo.toml and detach from workspace | ||
| run: | | ||
| import toml | ||
| cargo_toml = toml.load("Cargo.toml") | ||
| cargo_toml["dependencies"]["numpy"]["path"] = "../rust-numpy" | ||
| cargo_toml["dependencies"]["ndarray"] = "0.13.1" | ||
| cargo_toml["dependencies"]["num-complex"] = "0.2.4" | ||
| cargo_toml["workspace"] = {} | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like this 👍🏼
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Me too, but it was almost a necessity due to caching: The caching action would fail if its working directory was not present when run, hence building within an existing directory significantly simplified things compared to re-ordering steps so that the copy was created first, and then the caching action triggered. |
||
| with open("Cargo.toml", "w") as f: | ||
| toml.dump(cargo_toml, f) | ||
| working-directory: ../simple-extension-msrv | ||
| working-directory: examples/simple-extension | ||
| shell: python | ||
| - name: Use ndarray 0.13.1 | ||
| run: | | ||
| cargo generate-lockfile | ||
| cargo update -p $(cargo pkgid -p ndarray 2>&1 >/dev/null | grep 0.15 | sed -e 's/^[ \t]*//') --precise 0.13.1 | ||
| working-directory: ../simple-extension-msrv | ||
| - name: Test Example | ||
| working-directory: examples/simple-extension | ||
| - name: Test example | ||
| run: | | ||
| poetry install && poetry run maturin develop && poetry run pytest | ||
| working-directory: ../simple-extension-msrv | ||
| shell: bash | ||
| poetry install | ||
| poetry run maturin develop | ||
| poetry run pytest | ||
| working-directory: examples/simple-extension | ||
|
|
||
| linalg-example: | ||
| runs-on: ubuntu-latest | ||
|
|
@@ -127,11 +143,13 @@ jobs: | |
| uses: actions-rs/toolchain@v1 | ||
| with: | ||
| toolchain: stable | ||
| - name: Install maturin and poetry | ||
| run: pip install maturin poetry | ||
| - name: Test Examples | ||
| - uses: Swatinem/rust-cache@v1 | ||
| continue-on-error: true | ||
| - name: Install poetry | ||
| run: pip install poetry | ||
| - name: Test example | ||
| run: | | ||
| cd examples/linalg && \ | ||
| poetry install && \ | ||
| poetry run maturin develop && \ | ||
| poetry install | ||
| poetry run maturin develop | ||
| poetry run pytest | ||
| working-directory: examples/linalg | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,4 +14,5 @@ build/ | |
| *.so | ||
| *.egg-info/ | ||
| **/dist/ | ||
| __pycache__ | ||
| __pycache__ | ||
| poetry.lock | ||
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are jobs still parallelized without this? I haven't used actions recently so sorry if I'm asking something outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, they still run in parallel. But I would not have known, I had to try.