Skip to content

Conversation

@williampMSFT
Copy link
Contributor

@williampMSFT williampMSFT commented Jan 27, 2026

cargo test currently doesn't compile on Windows. It looks like this is because a couple crates have hard dependencies on libraries that don't work on desktop - in particular:

  • debug-service depends on defmt
  • espi-service depends on embassy-imxrt
  • partition-manager has an enabled-by-default dependency on defmt

These don't build on desktop, so when you try to run cargo test against mocks, the build fails.
We get away with it on Linux because it looks like the linker over there is more aggressive at pruning unused symbols than the MSVC linker, but the MSVC linker errors immediately when it can't find a referenced symbol, even if that symbol is not reachable from any entry points.

This change mitigates this problem by making partition-manager not default-depend on defmt and disabling build of debug-service and espi-service in test contexts. This does unfortunately mean that we can't write tests in those modules, but those modules already didn't have tests, so we're not conceding any existing test collateral by doing this.

In future, we can look into breaking the dependency espi-service has on embassy-imxrt by introducing traits. It's less clear to me how we would do this with the debug-service - perhaps a stub implementation of some of the defmt macros.

Fixes #691

Copilot AI review requested due to automatic review settings January 27, 2026 23:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes cargo test on Windows by avoiding desktop-incompatible dependencies from being pulled into workspace test builds.

Changes:

  • Remove defmt from partition-manager default features to prevent unintended desktop linking requirements.
  • Gate espi-service modules/exports behind #[cfg(not(test))] so workspace tests can compile on Windows.
  • Gate debug-service modules/exports behind #[cfg(not(test))] for the same reason, and adjust a few log/trace call sites in espi-service.

Reviewed changes

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

File Description
partition-manager/partition-manager/Cargo.toml Drops defmt from default features to avoid pulling it into desktop test builds.
espi-service/src/lib.rs Disables the crate’s modules/exports during tests to keep Windows test builds linkable.
espi-service/src/espi_service.rs Tweaks logging guards and error logging in the eSPI service implementation.
debug-service/src/lib.rs Disables the crate’s modules/exports during tests to keep Windows test builds linkable.

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

@williampMSFT williampMSFT marked this pull request as ready for review January 27, 2026 23:10
@williampMSFT williampMSFT requested review from a team as code owners January 27, 2026 23:10
@williampMSFT williampMSFT enabled auto-merge (squash) January 27, 2026 23:14
@williampMSFT williampMSFT disabled auto-merge January 28, 2026 17:47
@williampMSFT williampMSFT enabled auto-merge (squash) January 28, 2026 17:47
@williampMSFT williampMSFT merged commit a95d1ae into OpenDevicePartnership:v0.2.0 Jan 28, 2026
14 checks passed
@williampMSFT williampMSFT deleted the user/williamp/fix-test-build branch January 28, 2026 17:55
@jerrysxie jerrysxie added the bug Something isn't working label Jan 28, 2026
@jerrysxie jerrysxie linked an issue Jan 28, 2026 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Tests don't compile on Windows

5 participants