Skip to content

Conversation

danlaine
Copy link
Collaborator

@danlaine danlaine commented Sep 2, 2025

On main when I run cargo test -p commonware-resolver I get this compilation error:

➜  monorepo git:(main) ✗ cargo test -p commonware-resolver                      
   Compiling commonware-resolver v0.0.60 (/Users/XYZ/monorepo/resolver)
error[E0432]: unresolved import `crate::p2p::mocks`
   --> resolver/src/p2p/fetcher.rs:276:21
    |
276 |     use crate::p2p::mocks::Key as MockKey;
    |                     ^^^^^ could not find `mocks` in `p2p`
    |
note: found an item that was configured out
   --> resolver/src/p2p/mod.rs:48:9
    |
48  | pub mod mocks;
    |         ^^^^^
note: the item is gated behind the `mocks` feature
   --> resolver/src/p2p/mod.rs:47:7
    |
47  | #[cfg(feature = "mocks")]
    |       ^^^^^^^^^^^^^^^^^
error[E0432]: unresolved import `crate::p2p::mocks`
   --> resolver/src/p2p/wire.rs:110:21
 ...

The issue is that the commonware-resolver unit tests relies on the mocks feature, which is not enabled by default.

This PR enables the mocks feature for dev targets (i.e. unit tests) so cargo test -p commonware-resolver compiles and runs.


[dev-dependencies]
tracing-subscriber = { workspace = true }
commonware-resolver = { path = ".", features = ["mocks"] }
Copy link
Contributor

Choose a reason for hiding this comment

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

Alternative proposal, I suppose, is just to remove this mocks feature? I think it is pretty small in practice (and I'm not sure we take this sort of granular approach elsewhere yet).

Let's keep it and just see if we can avoid the test changes (I suspect this won't be the first time we need to do something like this).

@patrick-ogrady patrick-ogrady merged commit 2ad6031 into main Sep 3, 2025
54 of 55 checks passed
@patrick-ogrady patrick-ogrady deleted the danlaine/resolver-mocks branch September 3, 2025 21:39
Copy link

codecov bot commented Sep 3, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.06%. Comparing base (df8437d) to head (1ad7680).
⚠️ Report is 1 commits behind head on main.

@@           Coverage Diff           @@
##             main    #1527   +/-   ##
=======================================
  Coverage   92.06%   92.06%           
=======================================
  Files         283      283           
  Lines       73182    73182           
=======================================
+ Hits        67377    67378    +1     
+ Misses       5805     5804    -1     

see 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update df8437d...1ad7680. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants