Skip to content

tests(amdsmi): [AILITOOLS-308] Create python mock testing suite - #10555

Open
amd-josnarlo wants to merge 1 commit into
developfrom
users/josnarlo/Create_python_mock_testing_suite
Open

tests(amdsmi): [AILITOOLS-308] Create python mock testing suite#10555
amd-josnarlo wants to merge 1 commit into
developfrom
users/josnarlo/Create_python_mock_testing_suite

Conversation

@amd-josnarlo

Copy link
Copy Markdown
Contributor

Motivation

The Python suites that stub the amdsmi package installed fake modules into sys.modules and never restored them, so whichever suite ran next imported the stub and failed with ImportError: cannot import name 'AmdSmiRequiredCommandException' from 'amdsmi_cli_exceptions' (unknown location). The failure surfaced in an unrelated file and depended on discovery order, and the ad-hoc fixes applied so far left the same defect in three more classes with nothing to catch the next one. This groups the stubbing suites behind a single isolation contract and adds a guard that fails when any of them leaks.

Technical Details

Test harness (tests/python/common/common.py):

  • Added ModuleIsolationMixin — snapshots the sys.modules names listed in ISOLATED_MODULES and the sys.path entry in ISOLATED_PATH during setUpClass, restores both in tearDownClass
  • Added REPO_ROOT, resolved by walking up for amdsmi_cli/ + CMakeLists.txt; replaces the hard-coded ".." chains that needed editing on every directory move and silently SkipTested when off by one
  • run_test_dir now exits 2 when discovery yields zero tests and no -k filter is set, instead of reporting success on an empty suite

Mock tier (tests/python/unit/mock/{gpu,system}/):

  • Moved the seven suites that replace sys.modules entries under unit/mock/; the five that exercise the real library stay at unit/{gpu,system}/
  • Refactored all seven onto ModuleIsolationMixin, removing three per-file constant names, two hand-rolled snapshot/restore shapes, and every local tearDownClass
  • Closed the remaining leaks: TestCliMetricPartitionClock, TestCliMetricPartitionVirtualOS and TestVcnBusyNaviFallback never restored their amdsmi stubs; test_cli_set_clk_limit.py and test_output_file_stdin.py never removed their sys.path entry

Regression guard (tests/python/unit/test_module_isolation.py):

  • New. Runs the mock tier in a nested runner, then asserts every declared module is restored to the identical object, no undeclared stub survives, and sys.path is unchanged

Docs (tests/python/README.md, docs/conceptual/test-design.md, CHANGELOG.md):

  • Documented the unit/mock/ admission rule and the isolation contract a new suite must follow
  • Scoped the C++ "mocking is a technique, not a separate test level" rule to C++ and explained why Python needs a containment boundary for process-wide sys.modules state

Issue Tracking

JIRA ID : AILITOOLS-308

Test Plan

Run unit_tests.py

Test Result

Test results return as expected

Signed-off-by: josnarlo <joseph.narlo@amd.com>
@amd-josnarlo
amd-josnarlo requested a review from a team August 21, 2026 18:54
@github-actions github-actions Bot added documentation Improvements or additions to documentation project: amdsmi labels Aug 21, 2026
@therock-pr-bot

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass
Forbidden Files ✅ Pass
🧪 Unit Test ✅ Pass
🚫 Draft PR 🔜 To Be Enabled
🚩 Feature Flag 🔜 To Be Enabled
📊 Code Coverage 🔜 To Be Enabled

🎉 All policy checks passed!

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation organization: ROCm project: amdsmi

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant