Add nx_log! usage examples and unit tests for the log macro - #93
Conversation
|
Current head |
GianIac
left a comment
There was a problem hiding this comment.
Thanks for this PR.
Good coverage of the three formatting patterns and the doctest gating with #[cfg(target_arch = "wasm32")] is a nice touch.
- PR has conflicts with
main(likely from #90 adding#[inline]tolog::log). Please rebase and keep the#[inline]on the#[cfg(not(test))]version.
Question
- The issue asked for "docs + tests, no public API changes", but this replaces
log::logwith a fully separate#[cfg(test)]in-memory implementation. It works, but it's more invasive than needed and cascades into the#[expect(dead_code)]onhost_log_v2. - Would you consider a lighter approach? e.g. testing only the macro expansion via a small
#[cfg(test)] fn fmt_message(args: fmt::Arguments) -> Stringand asserting on that, without swapping outlog::logitself. That keepslog.rssingle-implementation and closer to the issue scope. - If you prefer to keep the current approach, that's OK too just add a short comment at the top of
log.rsexplaining why there are twolog::logdefinitions.
Nits (optional)
- Consider
pub(crate)ontake_test_messagesso future modules can assert on emitted logs. - The
assert_logged_messagehelper is nice, worth keeping either way.
Once rebased and we've agreed on the design direction, happy to merge.
Summary
nx_log!usageVerification
/Users/loi/.cargo/bin/cargo test -p nx-sdk/Users/loi/.cargo/bin/cargo doc -p nx-sdk --no-deps/Users/loi/.cargo/bin/cargo fmt --all -- --check/Users/loi/.cargo/bin/cargo clippy -p nx-sdk --all-targets -- -D warningsgit diff --checkCloses #67