Refactor cargo wdk action unit tests by krishnakumar4a4 · Pull Request #690 · microsoft/windows-drivers-rs

krishnakumar4a4 · 2026-06-25T07:06:07Z

Decouples BuildAction from BuildTask/PackageTask using mockable runner
abstractions (#[automock] + mockall_double), and replaces the monolithic
full-pipeline tests with focused, layered unit tests split across the
orchestration, build-task, and package-task layers.

Test migration at a glance

File	Layer	Before	After	Added	Deleted
`actions/build/tests.rs`	Orchestration	40	25	15	30
`actions/build/package_task.rs`	Package task	6	19	13	0
`actions/build/build_task.rs`	Build task	7	7	0	0

`tests.rs` — orchestration layer (40 → 25)

Deleted (30)

29 monolithic full-pipeline tests that drove the real BuildTask /
PackageTask end-to-end through mocked providers. Replaced by orchestration-only
tests plus dedicated task-layer unit tests:

29 given_*_when_*_then_* tests deleted.

1 arch-probe helper test replaced by a focused probe unit test:

parses_target_when_features_specified → replaced by
get_target_arch_from_cargo_rustc::probe_forwards_features (see Added)

Added (15)

14 orchestration-only tests that assert BuildAction wires the mock
BuildTaskRunner / PackageTaskRunner correctly, grouped into three submodules
by scenario:

standalone_driver_project

run_orchestrates_build_and_package_when_target_arch_is_detected
run_skips_probe_and_uses_explicit_target_arch
run_skips_package_runner_for_non_driver_package
run_skips_package_runner_when_no_cdylib_target
run_forwards_locked_and_features_to_metadata — asserts --locked and the
selected features are forwarded into the metadata provider, the build runner,
and the cargo rustc probe
run_returns_wdk_metadata_parse_error_for_invalid_metadata — invalid/partial
WDK metadata surfaces BuildActionError::WdkMetadataParse

driver_workspace

run_returns_workspace_error_when_one_member_fails
run_orchestrates_only_targeted_member
run_propagates_build_error_on_failure
run_returns_wdk_metadata_parse_error_for_conflicting_member_configs —
distinct per-member WDK configs surface WdkMetadataParse
run_returns_wdk_metadata_parse_error_for_conflicting_root_and_member_configs —
root vs member WDK config conflict surfaces WdkMetadataParse
run_builds_non_driver_only_workspace_from_root — non-driver-only workspace,
cwd = root: builds, packaging skipped, success
run_builds_non_driver_only_workspace_from_member — non-driver-only workspace,
cwd = member: builds, packaging skipped, success

emulated_workspace

run_builds_each_valid_project

1 probe helper unit test (replaces the deleted parses_target_when_features_specified):

get_target_arch_from_cargo_rustc

probe_forwards_features — asserts non-default features reach the cargo rustc
probe invocation

Renamed

The 8 original orchestration tests were renamed from their initial
given_*_when_*_then_* form to concise run_* names, since the enclosing
submodule now carries the "given" scenario context:

Submodule	Introduced as	Renamed to
`standalone_driver_project`	`given_a_driver_project_when_target_arch_is_detected_then_build_action_orchestrates_build_and_package`	`run_orchestrates_build_and_package_when_target_arch_is_detected`
`standalone_driver_project`	`given_explicit_target_arch_when_building_then_probe_is_skipped_and_package_runner_uses_it`	`run_skips_probe_and_uses_explicit_target_arch`
`standalone_driver_project`	`given_a_non_driver_package_when_building_then_package_runner_is_skipped`	`run_skips_package_runner_for_non_driver_package`
`standalone_driver_project`	`given_a_driver_package_without_cdylib_when_building_then_package_runner_is_skipped`	`run_skips_package_runner_when_no_cdylib_target`
`driver_workspace`	`given_a_workspace_root_when_one_member_fails_then_workspace_error_is_returned`	`run_returns_workspace_error_when_one_member_fails`
`driver_workspace`	`given_a_workspace_member_directory_when_building_then_only_that_member_is_orchestrated`	`run_orchestrates_only_targeted_member`
`driver_workspace`	`given_a_workspace_member_when_build_runner_fails_then_the_build_error_is_propagated`	`run_propagates_build_error_on_failure`
`emulated_workspace`	`given_an_emulated_workspace_when_running_then_each_valid_project_is_built`	`run_builds_each_valid_project`

Retained (10)

Unchanged unit tests for the two pure helpers, still in their existing submodules:

get_target_dir_from_output — unparsable_output_fails,
no_matching_artifact_fails, matching_artifact_without_dll_fails,
matching_dll_resolves_target_dir

get_target_arch_from_cargo_rustc — parses_amd64,
parses_arm64_with_whitespace_and_crlf, parses_arm64_with_internal_whitespace,
unsupported_arch_returns_error, missing_target_arch_returns_error,
invalid_utf8_returns_error

`package_task.rs` — package-task layer (6 → 19)

Added (13)

New PackageTask unit tests built with the PackageTaskHarness builder,
exercising the packaging pipeline in isolation from BuildAction:

run_packages_driver_with_expected_operations
run_verifies_signatures_when_enabled
run_skips_signing_and_verification_when_sign_mode_is_off
run_exports_certificate_from_store_when_it_already_exists
run_returns_error_when_inx_file_is_missing
run_returns_error_when_copying_driver_binary_fails
run_returns_error_when_stampinf_fails
run_returns_error_when_inf2cat_fails
run_returns_error_when_infverif_fails
run_returns_error_when_certmgr_fails
run_returns_error_when_makecert_fails
run_returns_error_when_signtool_fails
run_skips_infverif_for_samples_when_wdk_build_is_in_bugged_range

Retained (6)

new_succeeds_for_valid_args
new_panics_when_target_dir_is_not_absolute
new_panics_when_working_dir_is_not_absolute
stampinf_version_overrides_with_env_var
named_mutex::acquire_works_correctly
named_mutex::acquire_works_when_abandoned

Note: a module-level TEST_MUTEX serializes tests that call task.run() /
task.run_stampinf(). stampinf_version_overrides_with_env_var mutates the
process-global STAMPINF_VERSION env var, which previously caused
non-deterministic failures (e.g. run_verifies_signatures_when_enabled) when
tests ran in parallel.

`build_task.rs` — build-task layer (7 → 7, unchanged)

Test set is unchanged by this PR:

new_succeeds_for_valid_args
new_panics_when_working_dir_is_not_absolute
run_invokes_cargo_build_with_expected_args_and_returns_output
run_returns_command_failed_error_with_empty_stdout_when_cargo_build_exits_nonzero
run_returns_io_error_when_cargo_build_command_invocation_fails
run_forwards_locked_to_cargo_invocation_when_locked_is_set
run_forwards_features_to_cargo_invocation_when_features_are_set

Net effect

Orchestration tests are now fast, focused, and assert wiring only — task
internals (packaging steps, command-failure paths, signing) are covered at their
own layer.
Every removed full-pipeline behavior has an equivalent at the layer that owns it,
including WDK-metadata parse-failure propagation (asserted at the orchestration
layer) and every packaging command's failure path (asserted at the package
layer).
tests.rs shrank from ~3,680 to ~1,970 lines; package_task.rs test setup was
de-duplicated via shared harness helpers.
Build-action module test count: 53 → 51 (tests.rs 40→25, package_task.rs
6→19, build_task.rs 7→7), with packaging behavior now covered directly at the
package-task layer instead of through full-pipeline orchestration tests.

krishnakumar4a4 · 2026-06-25T07:07:57Z

@copilot resolve the merge conflicts in this pull request

Copilot

Pull request overview

This PR refactors cargo-wdk’s build/package orchestration to improve unit-testability by introducing injectable runner types for the build and package tasks, and rewrites the BuildAction unit tests to use those runners and more targeted expectations.

Changes:

Add BuildTaskRunner/BuildTaskRunParams and PackageTaskRunner wrappers to enable dependency injection and mocking.
Update BuildAction to accept runner instances (via new_with_runners) and route build/package work through the runners.
Replace large, end-to-end style BuildAction tests with a smaller orchestrator-focused suite + richer PackageTask unit tests.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.

File	Description
crates/cargo-wdk/src/actions/build/build_task.rs	Introduces `BuildTaskRunner` and parameter struct to decouple `BuildTask` execution for mocking.
crates/cargo-wdk/src/actions/build/mod.rs	Refactors `BuildAction` to use injected runners for build/package phases.
crates/cargo-wdk/src/actions/build/package_task.rs	Adds `PackageTaskRunner` and significantly expands/reshapes `PackageTask` unit test coverage.
crates/cargo-wdk/src/actions/build/tests.rs	Refactors `BuildAction` tests to validate orchestration via mocked runners and targeted expectations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

codecov-commenter · 2026-07-14T18:33:49Z

Codecov Report

❌ Patch coverage is 98.64662% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.84%. Comparing base (21ec8f8) to head (dcaf613).

Files with missing lines	Patch %	Lines
crates/cargo-wdk/src/actions/build/package_task.rs	98.56%	1 Missing and 8 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #690      +/-   ##
==========================================
+ Coverage   79.93%   81.84%   +1.90%     
==========================================
  Files          26       26              
  Lines        5633     6290     +657     
  Branches     5633     6290     +657     
==========================================
+ Hits         4503     5148     +645     
  Misses       1002     1002              
- Partials      128      140      +12

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

Copilot

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

Introduce BuildTaskRunner and PackageTaskRunner with #[automock] to decouple BuildAction orchestration tests from the full provider stack. BuildAction tests now mock runners instead of every individual provider, reducing tests.rs from ~3400 lines to ~700 lines while maintaining equivalent coverage through dedicated unit tests in build_task.rs and package_task.rs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fa978dd1-9d61-4cea-a03a-7ebc914228fa

…due to expectation overrides

Group top-level orchestration tests into three submodules by scenario type: - standalone_driver_project: single-package driver builds - driver_workspace: multi-package Cargo workspace builds - emulated_workspace: filesystem-based emulated workspace builds No test logic changes; only structural reorganization for clarity. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fa978dd1-9d61-4cea-a03a-7ebc914228fa

- Rename TestBuildAction to BuildActionHarness for consistency with PackageTaskHarness naming convention in the task layer. - Shorten test function names since submodule names already provide the 'given' context (e.g. standalone_driver_project::run_skips_probe...). - Rename test_build_action parameter to harness throughout. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fa978dd1-9d61-4cea-a03a-7ebc914228fa

BuildTaskRunner::run previously collected the entire cargo message stream into a Vec, which forced every message to be parsed and allocated up front and prevented get_target_dir_from_output from short-circuiting once the matching cdylib artifact is found. Return Box<dyn Iterator<..>> instead so the runner forwards BuildTask::run's lazy stream while still being mockable (mockall cannot mock an opaque impl Trait return, but can mock a boxed trait object). BuildTask::run gains + use<> precise capturing so its owned, 'static iterator can move out of the temporary BuildTask and be boxed. Addresses PR review comment r3581534373. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fa978dd1-9d61-4cea-a03a-7ebc914228fa

Copilot

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 9 comments.

…TaskRunner

Copilot

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 11 comments.

…in build action tests

Copilot

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated no new comments.

… off. This replaces the earlier removed test "given_a_driver_project_when_sign_mode_is_off_then_signing_and_verification_steps_are_skipped"

…ce metadata handling

…d enhancing readability

Copilot

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated no new comments.

…esses in PackageTaskRunner

Copilot

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings June 25, 2026 07:06

Copilot started reviewing on behalf of krishnakumar4a4 June 25, 2026 07:06 View session

Copilot AI reviewed Jun 25, 2026

View reviewed changes

Comment thread crates/cargo-wdk/src/actions/build/build_task.rs Outdated

Comment thread crates/cargo-wdk/src/actions/build/build_task.rs

Comment thread crates/cargo-wdk/src/actions/build/package_task.rs Outdated

This was referenced Jun 25, 2026

[WIP] Fix code based on review comments #692

Closed

[WIP] Fix code based on review comments #693

Closed

[WIP] Fix code based on review comments #694

Closed

Copilot AI review requested due to automatic review settings July 14, 2026 17:17

krishnakumar4a4 force-pushed the refactor-cargo-wdk-action-unit-tests branch from 2e2b890 to 7bc3ee6 Compare July 14, 2026 17:17

Copilot started reviewing on behalf of krishnakumar4a4 July 14, 2026 17:18 View session

krishnakumar4a4 force-pushed the refactor-cargo-wdk-action-unit-tests branch from 7bc3ee6 to 402e170 Compare July 14, 2026 17:21

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 14, 2026 17:22

Copilot started reviewing on behalf of krishnakumar4a4 July 14, 2026 17:23 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Comment thread crates/cargo-wdk/src/actions/build/build_task.rs Outdated

Copilot AI review requested due to automatic review settings July 14, 2026 18:06

krishnakumar4a4 force-pushed the refactor-cargo-wdk-action-unit-tests branch from 402e170 to e5eea7d Compare July 14, 2026 18:06

Copilot started reviewing on behalf of krishnakumar4a4 July 14, 2026 18:07 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Comment thread crates/cargo-wdk/src/actions/build/build_task.rs

Comment thread crates/cargo-wdk/src/actions/build/package_task.rs

Copilot AI review requested due to automatic review settings July 15, 2026 03:29

krishnakumar4a4 force-pushed the refactor-cargo-wdk-action-unit-tests branch from e5eea7d to b285c05 Compare July 15, 2026 03:29

Copilot started reviewing on behalf of krishnakumar4a4 July 15, 2026 03:31 View session

Copilot AI reviewed Jul 15, 2026

View reviewed changes

Comment thread crates/cargo-wdk/src/actions/build/tests.rs

Comment thread crates/cargo-wdk/src/actions/build/package_task.rs

Copilot AI review requested due to automatic review settings July 15, 2026 04:26

Copilot started reviewing on behalf of krishnakumar4a4 July 15, 2026 04:27 View session

Copilot AI reviewed Jul 15, 2026

View reviewed changes

Comment thread crates/cargo-wdk/src/actions/build/tests.rs

Comment thread crates/cargo-wdk/src/actions/build/tests.rs

Copilot AI review requested due to automatic review settings July 15, 2026 10:50

krishnakumar4a4 force-pushed the refactor-cargo-wdk-action-unit-tests branch from 7f6d686 to 9954a35 Compare July 15, 2026 10:50

Copilot started reviewing on behalf of krishnakumar4a4 July 15, 2026 10:51 View session

krishnakumar4a4 and others added 6 commits July 15, 2026 18:24

test: serialize test execution to prevent non-deterministic failures …

0bde358

…due to expectation overrides

refactor: simplify PackageTaskParams usage in run function

14ee218

Copilot AI review requested due to automatic review settings July 15, 2026 14:36

krishnakumar4a4 force-pushed the refactor-cargo-wdk-action-unit-tests branch from 9954a35 to c22da52 Compare July 15, 2026 14:36

Copilot started reviewing on behalf of krishnakumar4a4 July 15, 2026 14:36 View session

Copilot AI reviewed Jul 15, 2026

View reviewed changes

refactor: update dead code attributes for BuildTaskRunner and Package…

9d26684

…TaskRunner

Copilot AI review requested due to automatic review settings July 15, 2026 15:16

Copilot started reviewing on behalf of krishnakumar4a4 July 15, 2026 15:17 View session

Copilot AI mentioned this pull request Jul 15, 2026

[WIP] Fix code as per review comments in PR 690 #703

Closed

4 tasks

Copilot AI reviewed Jul 15, 2026

View reviewed changes

refactor: improve test mutex handling and enhance feature forwarding …

61a6884

…in build action tests

Copilot AI review requested due to automatic review settings July 16, 2026 04:05

Copilot started reviewing on behalf of krishnakumar4a4 July 16, 2026 04:06 View session

Copilot AI reviewed Jul 16, 2026

View reviewed changes

krishnakumar4a4 marked this pull request as ready for review July 16, 2026 04:11

krishnakumar4a4 added 3 commits July 16, 2026 12:21

refactor: add test to skip signing and verification when sign mode is…

540150b

… off. This replaces the earlier removed test "given_a_driver_project_when_sign_mode_is_off_then_signing_and_verification_steps_are_skipped"

refactor: add missing tests for WDK metadata parsing errors and enhan…

b64a1c5

…ce metadata handling

refactor: streamline packaging tests by consolidating expectations an…

4de28a3

…d enhancing readability

Copilot AI review requested due to automatic review settings July 16, 2026 08:48

Copilot started reviewing on behalf of krishnakumar4a4 July 16, 2026 08:49 View session

Copilot AI reviewed Jul 16, 2026

View reviewed changes

refactor: enhance test expectations for signing and verification proc…

dcaf613

…esses in PackageTaskRunner

Copilot AI review requested due to automatic review settings July 16, 2026 12:49

Copilot started reviewing on behalf of krishnakumar4a4 July 16, 2026 12:49 View session

Copilot AI reviewed Jul 16, 2026

View reviewed changes

Uh oh!

Conversation

krishnakumar4a4 commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test migration at a glance

tests.rs — orchestration layer (40 → 25)

Deleted (30)

Added (15)

Renamed

Retained (10)

package_task.rs — package-task layer (6 → 19)

Added (13)

Retained (6)

build_task.rs — build-task layer (7 → 7, unchanged)

Net effect

Uh oh!

krishnakumar4a4 commented Jun 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

krishnakumar4a4 commented Jun 25, 2026 •

edited

Loading

`tests.rs` — orchestration layer (40 → 25)

`package_task.rs` — package-task layer (6 → 19)

`build_task.rs` — build-task layer (7 → 7, unchanged)

codecov-commenter commented Jul 14, 2026 •

edited

Loading