-
Notifications
You must be signed in to change notification settings - Fork 6
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
Fix issues encountered during pytest #166
Changes from all commits
1afacc2
11b0162
229d4f0
4f6f2b8
9ca2e11
abd68a4
c5660d0
4a913ce
501b2c6
3cf1bd3
cee6005
1bdd93d
9608026
2f77822
1261f67
0573991
c86f6db
3d95985
e7a99b9
0823398
2184505
153cc5b
435f605
afc953a
b8a423c
c55a880
e3b200d
51bd2ef
f56d144
1d10eab
5051c11
5b274c2
706178f
8dace69
6dc3c23
8802214
d9238df
a25a117
a83a9eb
79d4ded
cfd75ed
ef074a6
83e16d4
9249823
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 |
---|---|---|
|
@@ -4,34 +4,111 @@ on: [push] | |
|
||
jobs: | ||
build-and-test: | ||
name: build ${{ matrix.python-version }} on ${{ matrix.platform || matrix.os }} | ||
name: build py3.${{ matrix.python-version }} on ${{ matrix.platform || matrix.os.base }}-${{ matrix.os.version }} | ||
strategy: | ||
fail-fast: true | ||
# NOTE: In the matrix below, we call out macos 13 and latest (> 13) explicitly. This is to enable running on both Intel and Apple Silicon for Mac. | ||
# As of this writing, MacOS 13 is Intel and MacOS 14 and above is Apple Silicon. Ideally, the Apple transition to their new chip occurs fast and | ||
# we can drop macos-13 checks and then just use 'latest'. | ||
matrix: | ||
os: | ||
- ubuntu | ||
- macos | ||
- windows | ||
- base: ubuntu | ||
version: latest | ||
- base: macos | ||
version: '13' | ||
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. @michael-okeefe , could you drop some comments in here explaining why 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. @michael-okeefe , please also explain in the comments what event should trigger adding 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. Sure, I can do that. The issue is related to what processor each github CI machine is running for mac. MacOS 13 is Intel and MacOS 14 is Apple Silicon. Ideally, the Apple transition to their new chip occurs fast and we can drop having to call out specific machines and then just use latest, etc. 15, 16, etc. can be brought in at any time we desire and they should look like the calls for 14. As soon as 13 drops from the board, we can go back to just saying 'latest'. 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. @calbaker Note added to clarify why we are calling out macos-13 and 14. |
||
arch: 'x86_64' | ||
- base: macos | ||
version: 'latest' | ||
arch: 'arm64' | ||
- base: windows | ||
version: latest | ||
arch: 'amd64' | ||
python-version: | ||
- "3.8" | ||
- "3.9" | ||
- "3.10" | ||
runs-on: ${{ format('{0}-latest', matrix.os) }} | ||
- "10" | ||
- "11" | ||
include: | ||
- os: | ||
base: ubuntu | ||
version: latest | ||
platform: linux | ||
- os: | ||
base: windows | ||
version: latest | ||
ls: dir | ||
runs-on: ${{ format('{0}-{1}', matrix.os.base, matrix.os.version) }} | ||
steps: | ||
- uses: actions/checkout@v4 | ||
|
||
- name: set up rust | ||
uses: dtolnay/rust-toolchain@stable | ||
- run: rustup target add aarch64-apple-darwin | ||
if: matrix.os == 'macos' | ||
|
||
- name: install rust target for MacOS 14 (Apple Silicon) | ||
run: rustup target add aarch64-apple-darwin x86_64-apple-darwin | ||
if: matrix.os.base == 'macos' && matrix.os.version == '14' | ||
|
||
- name: install rust target for MacOS 13 (Intel Mac) | ||
run: rustup target add x86_64-apple-darwin aarch64-apple-darwin | ||
if: matrix.os.base == 'macos' && matrix.os.version == '13' | ||
|
||
- name: Update minimum supported MacOS for Wheels for MacOS 13 (Intel Mac) | ||
run: echo "MACOSX_DEPLOYMENT_TARGET=10.12" >> $GITHUB_ENV | ||
if: matrix.os.base == 'macos' && matrix.os.version == '13' | ||
|
||
- name: run cargo tests | ||
run: cargo test | ||
|
||
- name: set up python | ||
uses: actions/setup-python@v5 | ||
uses: actions/setup-python@v4 | ||
# uses: actions/setup-python@v5 | ||
with: | ||
python-version: ${{ matrix.python-version }} | ||
python-version: "3.10" | ||
|
||
- name: install python dependencies | ||
run: python -m pip install --upgrade pip setuptools wheel pytest | ||
- name: run cargo tests | ||
run: cargo test | ||
run: python -m pip install --upgrade pip setuptools wheel twine cibuildwheel pytest | ||
|
||
- name: build source distribution | ||
if: matrix.os.base == 'ubuntu' && matrix.python-version == '10' | ||
run: | | ||
python -m pip install -U setuptools-rust | ||
python -c "import setuptools; setuptools.setup()" sdist | ||
|
||
- name: install FASTSim python | ||
run: pip install -e .[dev] | ||
run: python -m pip install -e .[dev] | ||
|
||
- name: run pytest | ||
run: pytest -v | ||
|
||
- name: build ${{ matrix.platform || matrix.os.base }} binaries | ||
run: cibuildwheel --output-dir dist | ||
env: | ||
CIBW_BUILD: "cp3${{ matrix.python-version }}-*" | ||
CIBW_SKIP: "*-win32 *-musllinux* *i686 *ppc64le *s390x *aarch64" | ||
CIBW_PLATFORM: ${{ matrix.platform || matrix.os.base }} | ||
# TODO: why doesn't pytest work with cibuildwheel? | ||
# CIBW_TEST_COMMAND: "pytest -v {project}/python/fastsim/tests" | ||
CIBW_TEST_COMMAND: "python -m unittest discover {project}/python/fastsim/tests" | ||
CIBW_ARCHS_MACOS: "${{ matrix.os.arch }}" | ||
# see https://cibuildwheel.readthedocs.io/en/stable/faq/#universal2 | ||
CIBW_TEST_SKIP: "*_universal2:arm64" | ||
CIBW_ENVIRONMENT: 'PATH="$HOME/.cargo/bin:$PATH"' | ||
CIBW_ENVIRONMENT_WINDOWS: 'PATH="$UserProfile\.cargo\bin;$PATH"' | ||
CIBW_MANYLINUX_X86_64_IMAGE: "manylinux2014" | ||
CIBW_MANYLINUX_I686_IMAGE: "manylinux2014" | ||
CIBW_BEFORE_BUILD: > | ||
pip install -U setuptools-rust && | ||
rustup default stable && | ||
rustup show | ||
CIBW_BEFORE_BUILD_LINUX: > | ||
yum -y install openssl openssl-devel && | ||
pip install -U setuptools-rust && | ||
curl https://sh.rustup.rs -sSf | sh -s -- --default-toolchain=nightly --profile=minimal -y && | ||
rustup show | ||
|
||
- name: list dist files | ||
run: ${{ matrix.ls || 'ls -lh' }} dist/ | ||
|
||
- uses: actions/upload-artifact@v4 | ||
with: | ||
name: fastsim-${{ matrix.platform || matrix.os.base }}-${{ matrix.os.version }}-py3.${{ matrix.python }}-${{ strategy.job-index }} | ||
path: ./dist/* | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,7 @@ authors = [{ name = "NREL/MTES/CIMS/MBAP Group", email = "[email protected]" }] | |
description = "Tool for modeling vehicle powertrains" | ||
readme = "README.md" | ||
license = { file = "LICENSE.md" } | ||
requires-python = ">=3.8,<3.11" | ||
requires-python = ">=3.10,<3.12" | ||
classifiers = [ | ||
"Programming Language :: Python :: 3", | ||
"License :: Other/Proprietary License", | ||
|
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.
@michael-okeefe , could we use
13
andlatest
so that we don't have to remember to increment the14
?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.
@calbaker Great idea. I made the change and the tests are running.