Repository navigation
amd-smi: Add AMD SMI KPI automation - #12701
Open
charis-poag-amd wants to merge 6 commits into
Open
charis-poag-amd wants to merge 6 commits into
charis-poag-amd wants to merge 6 commits into
Conversation
charis-poag-amd
requested review from
a team
and
a balanced review from Copilot
October 2, 2026 18:39
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Coverage accuracy is affected by stale logs, unstable CLI denominators, incorrect metadata, and flawed history handling.
Review effort: Balanced
Findings: 4
Open (6)
What changed in this PR
Adds AMD SMI API coverage automation, KPI reporting, trend history, and corrected test markers.
Changes:
- Adds API coverage execution, reporting, baselines, and CLI-surface measurement.
- Instruments C++ and Python tests with recognizable API markers.
- Documents coverage workflows, limitations, and generated artifacts.
| File | Description |
|---|---|
projects/amdsmi/tests/run_api_coverage.sh |
Runs suites and generates coverage artifacts. |
projects/amdsmi/tests/README.md |
Documents coverage tooling and metrics. |
projects/amdsmi/tests/python/functional/system/test_topology.py |
Fixes topology API markers. |
projects/amdsmi/tests/python/functional/nic/test_discovery.py |
Adds discovery API markers. |
projects/amdsmi/tests/python/functional/gpu/test_ptl.py |
Fixes PTL restore marker. |
projects/amdsmi/tests/python/functional/gpu/test_partition.py |
Fixes partition markers. |
projects/amdsmi/tests/python/functional/gpu/test_benchmark.py |
Standardizes benchmark markers. |
projects/amdsmi/tests/python/functional/cpu/test_power.py |
Adds CPU boost-limit marker. |
projects/amdsmi/tests/python/functional/cpu/test_benchmark.py |
Standardizes CPU benchmark markers. |
projects/amdsmi/tests/python/cli/test_reset.py |
Adds a redundant GPU-reset filter. |
projects/amdsmi/tests/cli_surface.py |
Measures the CLI command surface. |
projects/amdsmi/tests/api_coverage_report.py |
Generates KPI reports and trend history. |
projects/amdsmi/tests/api_coverage_baseline.csv |
Defines KPI baseline values. |
projects/amdsmi/tests/amd_smi_test/unit/system/status_string_test.cc |
Instruments status-string tests. |
projects/amdsmi/tests/amd_smi_test/unit/gpu/amdsmi_npm_limit_test.cc |
Instruments NPM API tests. |
projects/amdsmi/tests/amd_smi_test/functional/system/mutual_exclusion_test.cc |
Corrects initialization markers. |
projects/amdsmi/tests/amd_smi_test/functional/system/cross_process_serialization_test.cc |
Corrects process-role markers. |
projects/amdsmi/tests/amd_smi_test/functional/ifoe/fabric/fabric_read_test.cc |
Instruments fabric telemetry calls. |
projects/amdsmi/tests/amd_smi_test/functional/gpu/pci/pci_read_write_test.cc |
Instruments status conversion. |
projects/amdsmi/include/amd_smi/amdsmi.h |
Avoids parser-breaking punctuation. |
projects/amdsmi/.gitignore |
Ignores project-local coverage outputs. |
projects/amdsmi/.claude/rules/project-layout.md |
Documents new tooling locations. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
charis-poag-amd
added a commit
that referenced
this pull request
Oct 2, 2026
Addresses Copilot's review comments on #12701: - .gitignore: reverted the api-coverage-results/ ignore entries added earlier (both here and at the repo root) -- the default output dir isn't specific to running inside this repo, so we don't special-case this tool's scratch output in the repo's ignore files. - cli_surface.py: force is_baremetal/is_virtual_os (not just the device gates) so the measured CLI surface is stable regardless of which platform/host runs it; deliberately left is_hypervisor unforced since it's an unsupported configuration and combining it with is_amdgpu_initialized trips a pre-existing -d flag collision in the static subcommand's parser. - run_api_coverage.sh: clear known suite logs at the start of each run so reusing --out with a different suite selection (e.g. --no-cpp) can't silently count a previous run's stale log as current; added a real --no_history passthrough flag mirroring the existing -H handling. - test_reset.py: removed a leftover local-debugging TODO that temporarily filtered out --gpureset sweeps -- already handled centrally via TestCliBase.SWEEP_EXCLUDED_ARGS. - api_coverage_report.py: widened the [FAILED] regex in read_suite_status() to accept the Python suites' GTest-style wrapper's "N failure(s), listed below:" wording (real GTest says "N tests, listed below:"), which was silently zeroing the Failed column for any Python suite with real failures.
charis-poag-amd
force-pushed
the
users/charpoag/amdsmi-kpi-coverage-attribution-fixes
branch
from
October 2, 2026 21:29
737e701 to
1e05942
Compare
Adds self-contained tooling to measure API test coverage against the
public amdsmi.h surface, plus fixes for several bugs the tooling surfaced
in the test suite itself.
Tooling (tests/):
- run_api_coverage.sh: one-shot driver that runs every test suite, then
api_summary.py + api_coverage_report.py, producing a self-contained
./api-coverage-results-<timestamp>/ directory (report, CSVs, trend file).
- api_coverage_report.py: turns api_summary.py's per-API counts into the
KPI tables tracked on the AMD SMI KPI page (coverage by subsystem and by
framework), with a run metadata block and an appendable trend history.
- cli_surface.py: derives the CLI command-surface count by walking the
real argparse tree in amdsmi_cli/amdsmi_parser.py.
- api_coverage_baseline.csv: hand-maintained start/previous/goal values.
- .claude/rules/project-layout.md, .gitignore: document/ignore the above.
Bug fixes surfaced by the tooling (an API only counts as "Tested" when a
log line contains the literal marker `### <api>(`; these call sites
printed something that didn't match, so the APIs they exercise read
untested despite being called):
- amdsmi.h: reworded two amdsmi_get_npm_info() doc-comment sentences that
ended in `...);`, which made api_summary.py's naive @ingroup-to-`);`
parser mis-extract the following function's name as a multi-line string
containing comment prose -- corrupting _api_summary.csv and crashing
api_coverage_report.py with KeyError: 'Tested'.
- mutual_exclusion_test.cc, cross_process_serialization_test.cc: removed
`[role]` prefixes (e.g. `[sleeper] amdsmi_init(`) from DISPLAY_AMDSMI_API
calls that broke the literal `### amdsmi_` marker match; moved the role
tag to the desc argument instead.
- test_benchmark.py (cpu, gpu): fixed `_print_api_result()`'s `"### test
{func}("` prefix and `_log_test_start()`'s marker-less `"Testing {api}
on {device} {id}"` message (121 call sites in gpu, 54 in cpu).
- amdsmi_npm_limit_test.cc: added DISPLAY_AMDSMI_API markers to its 17
real calls into amdsmi_set_npm_limit()/amdsmi_get_npm_info()/
amdsmi_get_node_handle() (left the Python NPM unit tests and
rocm_smi_npm_test.cc uninstrumented -- the former fully mock the ctypes
call, the latter tests internal, non-public helpers not tracked by
amdsmi.h).
- status_string_test.cc, pci_read_write_test.cc, fabric_read_test.cc:
added markers for previously-uninstrumented real API calls.
- test_reset.py, test_power.py, test_partition.py, test_ptl.py,
test_discovery.py, test_topology.py: minor marker/message fixes in the
same vein.
Docs (tests/README.md):
- Added the two new marker failure modes above to the existing
"shapes that are silently invisible" examples.
- Noted the Unit column's numerator gap (most unit test files don't emit
the marker at all: 1/21 C++, 0/25 Python, checked 2026-10-02) alongside
the existing scale-placeholder caveat, with a mocked-call caveat.
- Added a "Related tooling in progress" section cross-referencing #9947
(line/branch/function code coverage -- a different metric) and #12494
(reusable unit-test fixtures, e.g. ScopedFileWatch, that NPM's ad hoc
TempBoardDir/ScopedAmdSmiInit helpers should migrate to once it lands).
Addresses Copilot's review comments on #12701: - .gitignore: reverted the api-coverage-results/ ignore entries added earlier (both here and at the repo root) -- the default output dir isn't specific to running inside this repo, so we don't special-case this tool's scratch output in the repo's ignore files. - cli_surface.py: force is_baremetal/is_virtual_os (not just the device gates) so the measured CLI surface is stable regardless of which platform/host runs it; deliberately left is_hypervisor unforced since it's an unsupported configuration and combining it with is_amdgpu_initialized trips a pre-existing -d flag collision in the static subcommand's parser. - run_api_coverage.sh: clear known suite logs at the start of each run so reusing --out with a different suite selection (e.g. --no-cpp) can't silently count a previous run's stale log as current; added a real --no_history passthrough flag mirroring the existing -H handling. - test_reset.py: removed a leftover local-debugging TODO that temporarily filtered out --gpureset sweeps -- already handled centrally via TestCliBase.SWEEP_EXCLUDED_ARGS. - api_coverage_report.py: widened the [FAILED] regex in read_suite_status() to accept the Python suites' GTest-style wrapper's "N failure(s), listed below:" wording (real GTest says "N tests, listed below:"), which was silently zeroing the Failed column for any Python suite with real failures.
…h API call The fabric read and write tests skipped their negative checks behind an IFoE gate, so those checks never ran on hosts without IFoE. Remove the gate, check the alloc/free nullptr cases before the telemetry section, and print the API name and status for every call (DISPLAY_AMDSMI_API and DISPLAY_AMDSMI_STATUS) so the KPI report counts them. The existing asserts are unchanged.
amdsmi_fabric_telem_id_to_string() is a static lookup that needs no device, so its tests belong in unit/ rather than in the functional IFoE read-only suite. Add IfoeUnit to the suite lists in test-design.md and tests/README.md.
… unit tests Print the API name and status for each call in the touched tests so the KPI report attributes the APIs to them. The asserts are unchanged.
charis-poag-amd
force-pushed
the
users/charpoag/amdsmi-kpi-coverage-attribution-fixes
branch
from
October 16, 2026 14:28
1e05942 to
7d525c7
Compare
github-actions
Bot
added
the
documentation
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds self-contained tooling to measure AMD SMI API test coverage against the public
amdsmi.hsurface (feeding the AMD SMI KPI page), plus fixes for several bugs the tooling itself surfaced in the test suite.Tooling (
tests/)run_api_coverage.sh— one-shot driver: runs every test suite, thenapi_summary.py+api_coverage_report.py, producing a self-contained./api-coverage-results-<timestamp>/directory (report, CSVs, trend file).api_coverage_report.py— turnsapi_summary.py's per-API counts into the KPI tables (coverage by subsystem, coverage by framework), with a run metadata block and an appendable trend history.cli_surface.py— derives the CLI command-surface count by walking the real argparse tree inamdsmi_cli/amdsmi_parser.py.api_coverage_baseline.csv— hand-maintained start/previous/goal values..claude/rules/project-layout.md,.gitignore— document/ignore the above.Bug fixes surfaced by the tooling
An API only counts as "Tested" when a log line contains the literal marker
### <api>(. The following call sites printed something that didn't match, so the APIs they exercise read untested despite being called:amdsmi.h: reworded twoamdsmi_get_npm_info()doc-comment sentences that ended in...);, which madeapi_summary.py's naive@ingroup-to-);parser mis-extract the following function's name as a multi-line string containing comment prose — corrupting_api_summary.csvand crashingapi_coverage_report.pywithKeyError: 'Tested'.mutual_exclusion_test.cc,cross_process_serialization_test.cc: removed[role]prefixes (e.g.[sleeper] amdsmi_init() fromDISPLAY_AMDSMI_APIcalls that broke the literal### amdsmi_marker match; moved the role tag to thedescargument instead.test_benchmark.py(cpu, gpu): fixed_print_api_result()'s"### test {func}("prefix and_log_test_start()'s marker-less"Testing {api} on {device} {id}"message (121 call sites in gpu, 54 in cpu).amdsmi_npm_limit_test.cc: addedDISPLAY_AMDSMI_APImarkers to its 17 real calls intoamdsmi_set_npm_limit()/amdsmi_get_npm_info()/amdsmi_get_node_handle(). Deliberately left the Python NPM unit tests androcm_smi_npm_test.ccuninstrumented — the former fully mock the ctypes call (no real API invocation to report), the latter tests internal, non-public helpers not tracked byamdsmi.h.status_string_test.cc,pci_read_write_test.cc,fabric_read_test.cc: added markers for previously-uninstrumented real API calls.test_reset.py,test_power.py,test_partition.py,test_ptl.py,test_discovery.py,test_topology.py: minor marker/message fixes in the same vein.Docs (
tests/README.md)ScopedFileWatch, that NPM's ad hocTempBoardDir/ScopedAmdSmiInithelpers should migrate to once it lands).Testing
_api_summary.csv/ReadAmdsmiHeader()parse cleanly (248/248 functions, no mangled keys) after theamdsmi.hfix.DISPLAY_AMDSMI_API/marker fix lands correctly viagrepagainst the actual source (not just tool-reported success).run_api_coverage.sh -Apass on an MI350 host: report generates without the priorKeyErrorcrash; System/Topo coverage went 75% → 100%, GPU 80% → 88% from these fixes alone.amdsmitst --gtest_filter=GpuUnit.*Npm*locally (non-root) and confirmed the instrumented calls print### amdsmi_set_npm_limit(...)/### amdsmi_get_npm_info(...)/### amdsmi_get_node_handle(...)as expected.