Conversation
There was a problem hiding this comment.
Pull request overview
Switches the project’s Python test suite from unittest/parameterized to pytest, updates automation/docs to run pytest, and adds a devcontainer setup.
Changes:
- Migrates existing tests to
pytest(fixtures + parametrization). - Updates CI/scripts/README to run
uv run pytestand addspytesttooling to dev dependencies. - Adds a VS Code devcontainer configuration (Dockerfile + settings).
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
uv.lock |
Locks new pytest-related dependencies (pytest, pytest-xdist, and transitive deps). |
pyproject.toml |
Adds pytest/xdist to dev deps and configures pytest options. |
test_py_cppmodel.py |
Converts unit tests from unittest.TestCase to pytest fixture-based tests. |
test_parse_standard_library_includes.py |
Replaces parameterized test expansion with pytest.mark.parametrize. |
test.macos.sh |
Updates macOS test command to run pytest. |
README.md |
Updates test instructions to use pytest. |
.github/workflows/python.yml |
Updates CI test step to run pytest. |
.devcontainer/devcontainer.json |
Adds devcontainer configuration and VS Code customizations. |
.devcontainer/Dockerfile |
Adds devcontainer image build instructions (Python + tooling). |
.devcontainer/.dockerfilelintrc |
Adds Dockerfile linter configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| && echo 'source /etc/bash_completion' >> ~/.bashrc | ||
|
|
||
| # Set up uv and Python environment | ||
| RUN curl -LsSf https://astral.sh/uv/install.sh | sh |
There was a problem hiding this comment.
Installing uv via curl ... | sh without pinning a version and verifying integrity is a supply-chain risk. For a devcontainer, consider installing a specific released version (and validating a checksum/signature) or using a package-based install method to make builds reproducible.
| RUN curl -LsSf https://astral.sh/uv/install.sh | sh | |
| RUN UV_INSTALL_VERSION="0.4.25" curl -LsSf https://astral.sh/uv/install.sh -o /tmp/uv-install.sh \ | |
| && sh /tmp/uv-install.sh \ | |
| && rm /tmp/uv-install.sh |
pyproject.toml
Outdated
| @@ -20,9 +20,14 @@ dev = [ | |||
| "jupyter>=1.1.1", | |||
| "parameterized>=0.9.0", | |||
There was a problem hiding this comment.
parameterized remains in the dev dependency group, but the updated test suite no longer imports/uses it. Removing it will reduce the dependency set and keep the lockfile smaller; re-generate uv.lock after dropping it.
| "parameterized>=0.9.0", |
| ] | ||
|
|
||
| [tool.pytest.ini_options] | ||
| addopts = "-n auto -v" |
There was a problem hiding this comment.
Setting addopts = "-n auto -v" makes pytest-xdist effectively mandatory for any pytest invocation (otherwise -n is an unknown option). Consider moving -n auto to CI/scripts (or using required_plugins to fail with a clearer message) so local/packaging workflows that install only pytest don’t break.
| addopts = "-n auto -v" | |
| addopts = "-v" |
| { | ||
| "name": "py-cppmodel Dev Container", | ||
| "build": { | ||
| "dockerfile": "Dockerfile", | ||
| "context": ".." | ||
| }, |
There was a problem hiding this comment.
The PR title is focused on switching the test framework, but this change also introduces a full devcontainer setup. Consider splitting the devcontainer addition into a separate PR to keep review scope tight and make the testing change easier to reason about.
| "source=${localEnv:HOME}/.ssh,target=/home/vscode/.ssh,type=bind,consistency=cached" | ||
| ], | ||
| "remoteUser": "vscode", | ||
| "updateRemoteUserUID": true, | ||
| "remoteEnv": { | ||
| "UV_LINK_MODE": "copy" |
There was a problem hiding this comment.
Mounting the host’s ~/.ssh into the container by default is a security risk (it exposes private keys to whatever runs in the container) and can be brittle across host OSes. Prefer SSH agent forwarding (SSH_AUTH_SOCK) or document an opt-in mount rather than enabling this by default.
| "source=${localEnv:HOME}/.ssh,target=/home/vscode/.ssh,type=bind,consistency=cached" | |
| ], | |
| "remoteUser": "vscode", | |
| "updateRemoteUserUID": true, | |
| "remoteEnv": { | |
| "UV_LINK_MODE": "copy" | |
| "source=${localEnv:SSH_AUTH_SOCK},target=/ssh-agent,type=bind" | |
| ], | |
| "remoteUser": "vscode", | |
| "updateRemoteUserUID": true, | |
| "remoteEnv": { | |
| "UV_LINK_MODE": "copy", | |
| "SSH_AUTH_SOCK": "/ssh-agent" |
No description provided.