Repository navigation
fix(skills): compile the package bound test module - #858
Merged
Merged
Conversation
The package bound tests live in `user_skills/package_tests.rs` because `tests.rs` crossed the rust file-size limit, but the declaration that compiles that module never reached the commit: the change was staged from a tree where a revert experiment had restored `user_skills.rs` with `git checkout --`. The file shipped undeclared, so its six tests never ran, the suite still reported 623 tests, and CI had nothing to fail on. Declare the module. The suite now reports 629 tests, six of them in `user_skills::package_tests`, and each bound has a test that fails when it is lowered.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#855 shipped
crates/host-core/src/user_skills/package_tests.rs— and never compiled it. The declaration#[cfg(test)] mod package_tests;was lost before the commit: the change was staged from a worktree where a revert experiment had restoreduser_skills.rswithgit checkout --, which discarded the uncommitted declaration along with the experiment.The file therefore shipped as an undeclared source file: its six bound tests never ran, the suite still reported 623 tests instead of 629, and CI had nothing to fail on. The production behavior of #855 is unaffected — the constants, the shared receiver bound, and the metadata pre-check all landed — but the record that pins them was dead weight.
Change
mod package_tests;inuser_skills.rs, the one line the commit was missing.Evidence
cargo test -p host-core --locked -- --listnow lists sixuser_skills::package_tests::*tests; before this fix it listed none.cargo test -p host-core --locked: 629 passed (623 before).two_devices…,…accepts_a_data_shaped_package,…accepts_resources_above_the_document_limit).engine_remote.rs→two_devices…fails withremote resource is too large.…accepts_a_data_shaped_packagefails.cargo fmt --checkandnode scripts/check-architecture.mjspass.Follow-up
CI cannot see a source file that no module declares. A "no orphan source file" rule would have caught this, but a quick prototype flagged legitimate modules (the
config_syncchild modules andrpc/*_tests.rs), so it needs a real design rather than a rushed rule. Until then, verification for host changes includes counting listed tests, not only running them.