-
Notifications
You must be signed in to change notification settings - Fork 44
Feat/dev 3737 matrix update #300
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 31 commits
c43c66f
a4dc9f3
09bf43d
6aed7a2
d3cae07
74214f0
2dc9fcd
f5939af
c35676b
284c234
6aacf13
e844d51
0335997
abb88a3
ab6aa2b
4ce7a3d
eb72e7f
3c39698
b312f1e
edad78b
4331cbf
0fb80f4
02fedb5
347511b
de5b35e
9bfd2d5
f41166c
a33a562
416644a
ceadbab
97c5375
ba2c0c2
af0257a
73cdce0
8a60af2
1d2b826
b798bc3
0e89471
d326913
ecb0e94
0dec662
af68af5
29fda5a
0265035
735551b
b8f4b89
16d595c
cd03bdc
4396843
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 |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ on: | |
| - develop | ||
| - release/** | ||
| - hotfix/** | ||
| - feat/DEV-3737_Matrix_update | ||
| tags: | ||
| - '*' | ||
|
|
||
|
|
@@ -14,15 +15,20 @@ jobs: | |
| strategy: | ||
| matrix: | ||
| os: | ||
| - macos-12 | ||
| - macos-13 | ||
| - macos-14 | ||
| - ubuntu-20.04 | ||
| - macos-15 | ||
| - macos-26 | ||
| - ubuntu-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. Maybe remove single non-deterministic one? |
||
| - ubuntu-22.04 | ||
| - windows-2019 | ||
| - ubuntu-24.04 | ||
| - windows-2022 | ||
| - windows-2025 | ||
| python: | ||
| - 3.8 | ||
| - "3.10" | ||
| - "3.11" | ||
| - "3.12" | ||
| - "3.13" | ||
| - "3.14" | ||
| env: | ||
| GDC_CLIENT_ZIP: 'gdc-client.zip' | ||
| steps: | ||
|
|
@@ -34,17 +40,21 @@ jobs: | |
| uses: actions/setup-python@v3 | ||
| with: | ||
| python-version: ${{ matrix.python }} | ||
| - name: Install uv | ||
| uses: astral-sh/setup-uv@v9.0.0 | ||
| - name: Install dependencies | ||
| run: uv sync --extra dev | ||
| env: | ||
| UV_DEFAULT_INDEX: https://pypi.org/simple | ||
| - name: Run Tests | ||
| if: matrix.os != 'windows-latest' | ||
| run: | | ||
| pip install --upgrade pip | ||
| pip install tox | ||
| tox -e py | ||
| env: | ||
| NO_PROXY: "127.0.0.1,localhost" | ||
| no proxy: "127.0.0.1,localhost" | ||
| run: uv run pytest tests/ | ||
| - name: Package | ||
| run: | | ||
| cd bin | ||
| pip install virtualenv | ||
| . ./package | ||
| uv run bash ./package | ||
| echo "GDC_CLIENT_ZIP=$GDC_CLIENT_ZIP" >> $GITHUB_ENV | ||
| shell: bash | ||
| - uses: actions/upload-artifact@v4 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,6 @@ | ||
| #!/usr/bin/env bash | ||
|
|
||
| set -euox pipefail | ||
|
|
||
| function cleanup() { | ||
| # get rid of the virtualenv | ||
| echo "Cleaning up" | ||
| deactivate | ||
| rm -rf "${VENV_NAME:?}/" build/ dist/ include/ local/ share/ gdc-client.spec | ||
| } | ||
|
|
||
| function compress() { | ||
| # this way it's cross platform | ||
| local zip_name=$1 | ||
|
|
@@ -24,76 +16,35 @@ with ZipFile('${zip_name}', 'w') as z: | |
|
|
||
| while getopts ":d" opt; do | ||
| case $opt in | ||
| d) | ||
| DATE=$(date "+%m_%d_%Y") | ||
| ;; | ||
| \?) | ||
| echo "Invalid option: -$OPTARG" >&2 | ||
| ;; | ||
| esac | ||
| d) DATE=$(date "+%m_%d_%Y") ;; | ||
| \?) echo "Invalid option: -$OPTARG" >&2 ;; | ||
| esac | ||
| done | ||
|
|
||
| DATE=${DATE-} # if date is not passed in, default DATE variable to empty string | ||
| VENV_NAME="venv" | ||
| BINARY_NAME="gdc-client" | ||
| VENV_PATH="$VENV_NAME/bin/activate" | ||
|
|
||
| case $(uname -s) in | ||
| *NT*) | ||
| # for use in git bash with all the nice unix utilities | ||
| TARGET_ENVIRONMENT="Windows" | ||
| BINARY_NAME="gdc-client.exe" | ||
|
|
||
| # this works in git bash | ||
| alias python="winpty python" | ||
|
|
||
| # why does windows have to be so different? | ||
| VENV_PATH="$VENV_NAME/Scripts/activate" | ||
|
|
||
| # Presumably Python3 is the only `python` in the Windows %PATH% variable | ||
| python -m virtualenv "$VENV_NAME" | ||
| source "$VENV_PATH" | ||
| pip install pywin32 | ||
| ;; | ||
| *Linux*) | ||
| TARGET_ENVIRONMENT="Ubuntu" | ||
| python -m virtualenv "$VENV_NAME" --python=python3 | ||
| source "$VENV_PATH" | ||
| ;; | ||
| *Darwin*) | ||
| TARGET_ENVIRONMENT="OSX" | ||
| python -m virtualenv "$VENV_NAME" --python=python3 | ||
| source "$VENV_PATH" | ||
| ;; | ||
| case "$(uname -s)" in | ||
| *NT*) TARGET_ENVIRONMENT="Windows"; BINARY_NAME="gdc-client.exe" ;; | ||
| *Linux*) TARGET_ENVIRONMENT="Ubuntu" ;; | ||
| *Darwin*) TARGET_ENVIRONMENT="OSX" ;; | ||
| *) echo "Unsupported OS"; exit 1 ;; | ||
| esac | ||
|
|
||
| # setup.py is in previous dir | ||
| cd .. | ||
|
|
||
| python -m pip install --upgrade pip | ||
| python -m pip install --no-deps -r requirements.txt | ||
| python -m pip install --no-deps . | ||
| uv sync --locked --extra dev | ||
|
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. Do we need the dev extras? If so these aren't getting packaged are they? |
||
|
|
||
| # go back go the ./gdc_client/bin dir | ||
| cd bin | ||
|
|
||
| echo "Building for ${TARGET_ENVIRONMENT}..." | ||
|
|
||
| # Make sure the correct version of pyinstaller and setuptools are installed | ||
| pip install -U PyInstaller | ||
|
|
||
| # Ran into ModuleNotFoundError: No module named 'pkg_resources.py2_warn' with higher versions of setuptools | ||
| pip install --upgrade setuptools | ||
|
|
||
| # Get version | ||
| VERSION=$(python -c " | ||
| VERSION=$(uv run python -c " | ||
| import gdc_client.version | ||
| print(gdc_client.version.__version__)") | ||
|
|
||
| echo "Creating ${TARGET_ENVIRONMENT} package for version ${VERSION}..." | ||
|
|
||
| # Create binary | ||
| pyinstaller \ | ||
| uv run --with pyinstaller pyinstaller \ | ||
| --clean \ | ||
| --additional-hooks-dir=. \ | ||
| --noconfirm \ | ||
|
|
@@ -118,7 +69,3 @@ ZIP_NAME="$PRE_ZIP_NAME.zip" | |
| echo "The output zip file will be called ${ZIP_NAME}" | ||
| compress "$ZIP_NAME" "$BINARY_NAME" | ||
| mv "$ZIP_NAME" .. | ||
| cd .. | ||
|
|
||
| cleanup | ||
| export GDC_CLIENT_ZIP="$PRE_ZIP_NAME" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,3 @@ | ||
| from importlib_metadata import version | ||
| from importlib.metadata import version | ||
|
|
||
| __version__ = version("gdc_client") |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,12 @@ | ||
| import hashlib | ||
| import hmac | ||
| import multiprocessing | ||
| import tarfile | ||
| import time | ||
| import urllib.error | ||
| import urllib.request | ||
| from collections.abc import Iterable, Mapping | ||
| from io import BytesIO | ||
| from multiprocessing import Process | ||
| from unittest.mock import patch | ||
|
|
||
| import boto3 | ||
|
|
@@ -141,11 +143,32 @@ def run_mock_server(): | |
|
|
||
| @pytest.fixture(scope="class") | ||
| def setup_mock_server() -> None: | ||
| server = Process(target=run_mock_server) | ||
| # MacOS does things differently, so get everyone acting the same way | ||
|
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. Was this something that changed for mac since py38?
Contributor
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 can find stuff online saying this change from fork to spawn for just MacOS happened for python 3.8. I don't know how this wasn't an issue for previous builds using python 3.8 on MacOS, but for the build matrix steps, this was causing issues for testing as part of the build process. |
||
| try: | ||
| ctx = multiprocessing.get_context("fork") | ||
| server = ctx.Process(target=run_mock_server) | ||
| except ValueError: | ||
| server = multiprocessing.Process(target=run_mock_server) | ||
|
|
||
| server.start() | ||
| time.sleep(5) # starting with py38, takes longer for process to start on macOS | ||
|
|
||
| # Since py38, a sleep is needed for MacOS. 10 is no longer enough as of py310 | ||
| # Instead of time.sleep(30), this loop could exit faster. | ||
|
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. Are you seeing a significant reduction in the wait time for this? This increases complexity a bit and I might recommend just going with the 30 sec sleep if we're only saving a couple seconds.
Contributor
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 can tell a difference on my machine when running sleep(30) instead of this loop, but how often does a person run the tests? Maybe taking longer and being less complex would be preferred |
||
| for _ in range(60): | ||
| try: | ||
| with urllib.request.urlopen("http://127.0.0.1:5000", timeout=1): | ||
| break | ||
| # listen for any response, and then stop waiting | ||
| except urllib.error.HTTPError: | ||
| break | ||
| except Exception: | ||
| time.sleep(0.5) | ||
| else: | ||
| raise RuntimeError("Mock server failed to start on 127.0.0.1:5000 in 30 secs.") | ||
|
|
||
| yield | ||
| server.terminate() | ||
| server.join() | ||
|
|
||
|
|
||
| @pytest.fixture | ||
|
|
||
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.
We shouldn't check this in.
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.
This line will be removed at the end, but great call out. I tried to make a comment that would block this merge until this line is deleted, but I couldn't find that option quickly enough so I just kept on moving.