Repository navigation
test(amdsmi): packaging install-path and multi-Python-version coverage - #9034
Conversation
0062799 to
2aff5a5
Compare
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
4fce49e to
3645f9e
Compare
2aff5a5 to
4c1eca0
Compare
3645f9e to
4eaeeeb
Compare
4c1eca0 to
634216d
Compare
634216d to
f8f4676
Compare
f12f477 to
8b8852f
Compare
f8f4676 to
4d43287
Compare
8b8852f to
b580ad8
Compare
2feb4ee to
99e4ead
Compare
b03bbc6 to
7843436
Compare
99e4ead to
ba51dca
Compare
7843436 to
22ce26e
Compare
ba51dca to
62c51b2
Compare
22ce26e to
d5ff51f
Compare
62c51b2 to
ff9dd17
Compare
d5ff51f to
7dc2998
Compare
ff9dd17 to
560ef0a
Compare
7dc2998 to
7e2e902
Compare
560ef0a to
b30ba1d
Compare
7e2e902 to
799bf48
Compare
b30ba1d to
2deff96
Compare
799bf48 to
10d1d26
Compare
2deff96 to
6410cd3
Compare
954c19a to
07fc1f0
Compare
07fc1f0 to
5951aa7
Compare
There was a problem hiding this comment.
🟡 Changes recommended
There are a few correctness/robustness issues in the new Python-version and upgrade/downgrade test harnesses that can cause misleading failures or missed coverage.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Adds CI and harness coverage to validate the AMD SMI Python packaging rework across install location correctness, multiple CPython versions, and package upgrade/downgrade transitions.
Changes:
- Introduces standalone packaging validation scripts for cpack install-path inspection, multi-interpreter loader tests, and upgrade/downgrade round-trip checks.
- Wires the new checks into the existing
amdsmi_buildtest harness with new CLI options. - Adds dedicated GitHub Actions workflows for Python-version testing and package upgrade/downgrade validation.
File summaries
| File | Description |
|---|---|
| projects/amdsmi/tests/run_amdsmi_upgrade_downgrade_test.py | New script to install old/new packages and verify import, CLI execution, and linker config across upgrade/downgrade. |
| projects/amdsmi/tests/run_amdsmi_python_versions_test.py | New script to run loader/contract tests under multiple CPython interpreters. |
| projects/amdsmi/tests/run_amdsmi_cpack_path_test.py | New script to inspect built .deb/.rpm contents and assert the module lands under site/dist-packages. |
| projects/amdsmi/tests/amdsmi_build/run_amdsmi_build.py | Integrates new verification steps and adds CLI flags for upgrade-from artifact + interpreter list. |
| .github/workflows/amdsmi-upgrade-downgrade.yml | New workflow to build two package versions and validate upgrade/downgrade behavior in-container. |
| .github/workflows/amdsmi-python-versions.yml | New workflow to run loader tests across a Python version matrix (including container legs for 3.6/3.7). |
Review details
Suppressed comments (1)
projects/amdsmi/tests/run_amdsmi_python_versions_test.py:85
- The CLI help text says the default discovery is python3.6..3.13, but the workflow matrix includes 3.14; this will become misleading after expanding discovery to include 3.14.
help="Python interpreters to test (default: discovered python3.6..3.13).",
- Files reviewed: 6/6 changed files
- Comments generated: 4
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
79b347b to
eeeca40
Compare
Packaging install-path and multi-Python-version test coverage for the AMD SMI Python packaging rework. - Loader across Python versions: verify loader behavior across supported interpreters; run the 3.6/3.7 loader tests in python container images. - Install-path assertion: assert the system package ships the module on an importable path (site-packages/dist-packages). - Upgrade/downgrade round trip: exercise the deb/rpm upgrade and downgrade path in CI; select upgrade-test packages by build order (not version string); fail the upgrade test when the ld.so.conf.d linker registration is missing; run the CLI check in the upgrade test. - Python 3.6 compatibility: make the version and package tests run on Python 3.6. - CI hygiene: wire the packaging tests into the harness and CI, sparse-checkout the python-version and upgrade jobs, pin actions/setup-python to a commit SHA. JIRA ID: ROCM-3941
eeeca40 to
4c188e1
Compare
|
Flaky github fetch sources error, build and test for smi are good |
## Description Validate the SLES RPM interpreter dependency in CI. Stacks on #9034 (it invokes `run_amdsmi_cpack_path_test.py`, which #9034 adds). - Add a SLES packaging job that builds the RPM and asserts the interpreter dependency resolves to a real SLES package (`pythonXY`, not the RHEL `python(abi) = 3.X` or bare `python3` forms). - Give the SLES job a working toolchain and dependency check; match only the SLES `pythonXY` token in the dependency check; sparse-checkout the SLES packaging job. - Bound the job with an explicit `timeout-minutes` so a hung container fails fast instead of inheriting the six-hour default. ## JIRA ID ROCM-3941 --------- Co-authored-by: Galantsev, Dmitrii <dmitrii.galantsev@amd.com>
…ation (#9695) Stacked on #9110 — this targets that branch, not `develop`. ``` develop └─ #9034 packaging install-path and multi-Python coverage └─ #9035 SLES RPM interpreter dependency check └─ #9110 component scriptlet isolation └─ #9695 (this PR) ``` ## What this is The regression tests #9110 didn't ship with, plus fixes for several unrelated defects in the packaging test suite that let real breakage through. Nothing here changes #9110's fix — this PR adds tests and CI only, apart from the two behavior changes called out at the bottom. ## Regression coverage for #9110 #9110 stops `amd-smi-lib-tests` from tearing down the still-installed `amd-smi-lib`. Two tests pin it: - **`test_packaging_scriptlets.py`** — static guard on the templates and the CPack wiring. Written against the *invariant* rather than one particular remedy, so it accepts either "give the tests component its own scriptlets" or "scope the globals to the owner". - **`run_amdsmi_component_removal_test.py`** + a workflow step — installs both packages in the distro container, removes only the tests package, and asserts the main package survives: still installed, linker registration intact, module still importable and still owned by the main package, log directory and logrotate config untouched. The static guard alone is not sufficient. It can only confirm which CPack variables were *written*, never that CPack *honors* them, so it passes against packaging that is genuinely broken. The container test is what closes that gap. ## Also fixed — the test oracles themselves - **CPack path check** accepted any path merely *containing* `site-packages`. Verified false positives on real synthetic packages: `/usr/lib/python3/not-site-packages/…`, `/opt/acme/site-packages/…`, and a wrong Python minor. It now matches against the target interpreter's `site.getsitepackages()`, and picks that interpreter the way packaging does (`/usr/libexec/platform-python`, then `/usr/bin/python3`) rather than `sys.executable`, which differs on the RHEL family. It also parses `dpkg -c` without mangling symlink entries or paths containing spaces. - **Upgrade round trip** verified a bare `import amdsmi`. A pip wheel under `/usr/local` shadows the packaged module, so all three phases could validate the *same unchanged wheel* while the system package really did change — reproduced on Ubuntu 22.04 and AlmaLinux 9. It now requires the imported module to be owned by the package under test, comparing the owning package name **exactly**: a substring test accepts `amd-smi-lib-tests`, whose name contains the main package's. OLD and NEW must also differ, so equal versions can't no-op past the test. - **Build harness** ran that round trip *after* installing the wheel; moved ahead of the install and wheel stages. - **Post-install** now warns, non-fatally and destroying nothing, when a pip-installed `amdsmi` shadows the packaged module. - **`test_run_amdsmi_build.py`** never registered the module in `sys.modules` before `exec_module`, which `@dataclass` requires on Python 3.12+. All 16 of its tests were uncollectable. ## Testing The new unit tests are hermetic — no root, Docker, network or GPU — and run on Python 3.6 through 3.14 via the version harness. Both new guards fail on the pre-fix code for the right reason, not merely because a helper is missing: the scriptlet guard fails on the pre-fix templates, and the CPack path guard drives the old CLI through a fake `dpkg` shim and reports the unimportable path it used to accept. Each commit is self-contained and was verified individually, so the stack bisects cleanly. ## Notes for review - Two behavior changes only: the post-install warning, which deletes nothing and always exits 0, and stage ordering inside a test harness. Everything else is test coverage or CI. - The upgrade-round-trip ownership check uses exact package-name comparison rather than the `"amd-smi-lib"` prefix suggested in review, because that string is also a prefix of `amd-smi-lib-tests`. The sibling case is covered by a test. ## JIRA ID ROCM-3941
## Motivation Python 3.15.0 is out, but the AMD SMI Python-version tests stop at 3.14, so a loader or wrapper change that only breaks on 3.15 isn't caught. ## Technical Details **Workflow** (`.github/workflows/amdsmi-python-versions.yml`): - Add a `3.15` leg. It runs like the 3.8 to 3.14 legs, on the GitHub-hosted `ubuntu-22.04` runner via `setup-python`, which has a stable 3.15.0 build for it **Tests** (`projects/amdsmi/tests/`): - `run_amdsmi_python_versions_test.py`: raise `NEWEST_SUPPORTED_MINOR` to 15, which its comment says to keep in sync with the matrix - Move the `3.6..3.14` range to 3.15 in the harness and `run_amdsmi_build.py` help text and a `test_abi_compat.py` docstring ## Issue Tracking Extends the matrix added in #9034. ## Test Plan - Ran the workflow's 3.15 test command against this branch with the exact build `setup-python` installs (`python-3.15.0-linux-22.04-x64`), in an offline `ubuntu:22.04` container - Imported `amdsmi_wrapper` on 3.15 with all warnings enabled ## Test Result - Python 3.15.0 passes the loader, ABI-compat, and packaging guard tests - No warnings on 3.15 ## Submission Checklist - [ ] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md. Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
Description
Packaging install-path and multi-Python-version test coverage for the AMD SMI Python packaging rework. Stacks on #9033 (packaging correctness follow-ups).
site-packages/dist-packages).ld.so.conf.dlinker registration is missing; actually run the CLI check in the upgrade test.actions/setup-pythonto a commit SHA.JIRA ID
ROCM-3941