[idass1990]

Summary

Adds a standalone, dispatchable install test for the Windows MSI packages (Milestone 5 of #1987), mirroring the Linux native-package install-test pattern per ScottTodd's guidance: it consumes an arbitrary build run's MSIs, is callable by ci/release, and is runnable locally for iteration.

• native_windows_package_install_test.py — downloads each package's MSI from the public {run_id}-windows/packages/msi/ prefix (filenames derived from the generator's PACKAGES table, so no S3 listing and no credentials — fork-friendly), installs via msiexec /i /qn, and verifies the install directory, payload DLLs, System32 legacy DLLs, and the HKLM registry key. Dual CLI + pytest + env-var entry like the Linux script; --msi-dir skips the download for local iteration against a freshly built MSI.
• native_windows_package_uninstall_test.py — looks up the ProductCode by DisplayName, runs msiexec /x /qn, and verifies the install dir + registry key are gone (shared, ref-counted System32 DLLs are intentionally not asserted).
• test_native_windows_packages_install.yml — workflow_dispatch + workflow_call on windows-2022, public download (no creds/guard → forks run it), conditional uninstall step gated on run_uninstall. actionlint- and zizmor-clean.
• Unit tests for both scripts (mock msiexec/winreg/downloads/filesystem) run on any platform; the live modules import winreg lazily and the pytest entries skip on non-Windows, so Linux whole-tree collection stays green.

Style compliance

Audited against docs/development/style_guides/: argparse + type hints + pathlib, specific exceptions (no bare sys.exit), SHA-pinned actions, inputs passed via env: (no template injection), safe release_type dispatch default (choice [ci, dev], matching the Linux sibling), set -euo pipefail. Test filenames are globally unique (whole-tree pytest).

Test plan

• Unit tests: pytest build_tools/packaging/windows/tests/ — install + uninstall UTs pass on Linux (33 tests).
• actionlint + zizmor --offline clean on the new workflow.
• End-to-end verified on a real MSI. Dispatched the build workflow on ROCm (run 36895974522, release_type=ci) to produce amdrocm-runtime.msi in the public ci bucket, then dispatched this install test against it:
  https://github.com/idass1990/TheRock/actions/runs/36896581592 — ✅ green
  • Install + verify: install directory present: C:\Program Files\AMD\ROCm\core-10.2, payload DLLs present, System32 DLLs present, registry key present: HKLM\Software\AMD\ROCm\runtime\10.2.
  • Uninstall + teardown: ProductCode looked up by DisplayName, install directory removed, registry key removed.

Notes

• The install test consumes the MSIs the build workflow uploads to the artifacts bucket (still present). It is independent of the release-bucket publish that was reverted in Revert MSI publishing from the release publish script (#8617) #8629.
• Fork caveat: a fork build run does not upload MSIs (if: owner == 'ROCm'), so live-testing requires a real ROCm build run's artifact_run_id (as done above). Local --msi-dir needs no S3 at all.

Follow-ups (out of scope): --test-type modes, smoke-running a bundled exe (needs core pkg/driver/GPU), and wiring build + test into ci/release (Milestone 4).

[therock-pr-bot]

✅ All Checks Passed — Ready for Review

Check	Status	Details
📝 PR Description	✅ Pass	—
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	✅ Pass	—
🔎 pre-commit	✅ Pass	—
🚫 Draft PR	🔜 To Be Enabled	—
🚩 Feature Flag	🔜 To Be Enabled	—
📊 Code Coverage	🔜 To Be Enabled	—
🤖 therock-pr-bot	✅ Pass	—

🎉 All checks passed! This PR is ready for review.

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

🙋 Wish to Override Policy?

[therock-pr-bot]

🎉 All checks passed! This PR is ready for review.

[ScottTodd]

Quick surface level review, dumping state before end of week.

I also reviewed with an agent using this prompt:

› Please review #8669. In particular:

• github actions style / security, e.g. set -euo pipefail shouldn't be needed since github applies similar modes to bash run: steps by default
• why do the tests themselves have tests? did we do something similar for native linux packaging...? that seems very weird
• workflow design and location iteration: the build_tools/packaging/windows/native_windows_package_install_test.py script downloads the packages. What about running the test locally? I would prefer to download once and then be able to run and re-run each test using what I've downloaded
• are the tests themselves meaningful? what potential issues are they guarding against? do they miss important cases?

which produced https://github.com/ScottTodd/rocm-workspace/blob/main/reviews/pr_TheRock_8669.md

it found a few notable gaps in there which I haven't verified:

• Uninstall selects a product without validating the requested version
• The payload test does not establish that each selected package installed its payload
• Registry verification accepts a wrong installation location

[idass1990]

Quick surface level review, dumping state before end of week.

I also reviewed with an agent using this prompt:

› Please review #8669. In particular:

• github actions style / security, e.g. set -euo pipefail shouldn't be needed since github applies similar modes to bash run: steps by default
• why do the tests themselves have tests? did we do something similar for native linux packaging...? that seems very weird
• workflow design and location iteration: the build_tools/packaging/windows/native_windows_package_install_test.py script downloads the packages. What about running the test locally? I would prefer to download once and then be able to run and re-run each test using what I've downloaded
• are the tests themselves meaningful? what potential issues are they guarding against? do they miss important cases?

which produced https://github.com/ScottTodd/rocm-workspace/blob/main/reviews/pr_TheRock_8669.md

it found a few notable gaps in there which I haven't verified:

• Uninstall selects a product without validating the requested version
• The payload test does not establish that each selected package installed its payload
• Registry verification accepts a wrong installation location

the native linux packaging does indeed unit test its tests for install and uninstall so this was modelled after that for consistency sake.. However, I do see your point on how this is not the cleanest approach and indeed is a product of excess logic in tests. I will refactor this following the "keep logic out of tests" paradigm

[idass1990]

Restructured per the review. Marked draft: this now depends on #8723 (shared registry discovery key) and #8724 (System32 opt-in) — the tests import discovery_registry_key from the generator and assert the new behavior, so the module won't import standalone until those land. Will rebase and mark ready once both merge.

How the review points were addressed:

• No network in tests / download once, re-run. New download_msi_packages.py downloads a run's MSIs into a directory; the tests consume them via --msi-dir / MSI_DIR and never touch the network. The workflow has a separate "Download MSI packages" step, so CI and developers download once and re-run the tests against the same files.

• No tests-for-tests. Reusable logic moved into generate_msi_wxs.py with its own unit tests (parse_major_minor, expected_msi_filename, install_subdir_for, discovery_registry_key). Both native_windows_package_*_ut_test.py files are deleted; real behavior is exercised by the live install test on a Windows runner.

• Uninstall version validation. Uninstall resolves the ProductCode, then confirms that product's recorded InstallLocation (or its component files) belongs to the requested version's install directory before removing it. Verified: uninstalling --rocm-version 9.9.0 while 10.2 is installed is refused.

• Package-specific payload. The payload check resolves each package's ProductCode and asks Windows Installer which files it installed (MsiEnumComponents / MsiGetComponentPath via ctypes), then requires those files to exist under the install dir — so one package's files can't satisfy another's check. Packages are validated standalone.

• Registry path validation. The discovery-key check compares InstallDir against the expected install directory and requires it to exist, rather than only checking non-empty.

• Minor: msiexec 3010 (reboot required) is treated as success; dropped set -euo pipefail from the workflow run: steps; aligned the test workflow's release_type default with the MSI build workflow (ci).

Verified end-to-end on Windows against a real MSI built from the combined (#8723 + #8724) generator: install (7 payload files found via the Installer API, System32 scrubbed, discovery key InstallDir/Version match), uninstall + teardown, and the negative wrong-version case all behave as expected.

[idass1990]

Rebased onto main now that #8723 and #8724 have merged, and marked ready for review.

• Imports resolve (the reason this was draft): the tests now import discovery_registry_key from the merged packaging: use a shared versioned registry key for ROCm discovery (RFC0014) #8723 — Unit Tests pass on both Ubuntu and Windows.
• Conflict resolution kept both packaging: use a shared versioned registry key for ROCm discovery (RFC0014) #8723's discovery_registry_key and this PR's helpers (parse_major_minor, expected_msi_filename, install_subdir_for) in generate_msi_wxs.py.
• Updated two docstrings to the renamed LEGACY_SYSTEM32_INSTALL property (from packaging: make System32 DLL install opt-in and scrub stale copies #8724).
• verify_system32_absent already asserts the default install removes the DLLs from System32, matching packaging: make System32 DLL install opt-in and scrub stale copies #8724's behavior — no change needed.

89 packaging tests pass; pre-commit / pr-bot green.