Add package-boundary smoke test (#411) - #469
Conversation
Guard the packaged crate's manifest against omitted build-script sources and stale inlined `ninja_env` paths before a release exposes them.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
Summary
WalkthroughAdd a packaging smoke test that runs Cargo publication and packaging commands, verifies required build-script sources remain packaged, rejects stale ChangesPackaging boundary validation
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 20✅ Passed checks (20 passed)
📋 Issue PlannerLet us write the prompt for your AI agent so you can ship faster (with fewer bugs). View plan for ticket: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Reviewer's GuideAdds a packaging smoke test that runs Flow diagram for packaging smoke test using cargo package --listflowchart LR
A[Run test packaging_smoke_tests.rs] --> B[Invoke cargo package --list]
B --> C[Assert build_l10n_audit.rs is listed]
B --> D[Assert build script sources are listed]
B --> E[Assert ninja_env paths are not listed]
C --> F[Pass if all assertions hold]
D --> F
E --> F
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9c034cdc0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| rather than `cargo publish --dry-run` because the unversioned `ambient_fs` path | ||
| dependency has `publish = false`, which blocks a full publish verification. |
There was a problem hiding this comment.
Remove the obsolete ambient_fs packaging rationale
In the reviewed tree, a repo-wide search for ambient_fs finds only this paragraph, and Cargo.toml no longer declares that dependency because commit 576e421 removed it before this change. The guide therefore gives contributors a false reason that full publish verification is unavailable; re-evaluate whether cargo publish --dry-run can now replace --list, or document the actual remaining blocker.
AGENTS.md reference: AGENTS.md:L44-L47
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@coderabbitai Has this now been resolved in the latest commit?
Use codegraph analysis to determine your answer.
If this comment is now resolved, please mark it as such using the API. Otherwise, please provide an AI agent prompt for the remaining work to be done to address this comment.
There was a problem hiding this comment.
@coderabbitai Addressed and pushed in commit 954ed7e.
packaging_smoke_tests.rs now requires
cargo publish --dry-runto succeed, then usescargo package --listfor exact manifest assertions.developers-guide.md no longer mentions
ambient_fs.- All local Rust and documentation gates passed.
- PR #469 was updated with the stronger test contract.
- The original thread is outdated. Remote CodeRabbit was still processing at the final check, with no new inline findings posted.
There was a problem hiding this comment.
@leynos, resolved. The updated contract now runs cargo publish --dry-run and
uses cargo package --list only for exact packaged-manifest assertions; the
obsolete ambient_fs blocker rationale has also been removed.
Run `cargo publish --dry-run` before inspecting the packaged file list so the standard test suite verifies that the release artefact builds. Remove the obsolete `ambient_fs` limitation from the developer guide and describe the distinct publish-verification and manifest-inspection steps.
|
@coderabbitai review |
✅ Action performedReview finished.
|
Reject only exact `ninja_env` path components so legitimate packaged files with longer names are not mistaken for stale inlined-crate paths.
|
@coderabbitai review |
✅ Action performedReview finished.
|
Summary
This branch adds a release-boundary smoke test that verifies the packaged crate
builds successfully with
cargo publish --dry-run, retains requiredbuild-script sources and excludes stale
ninja_envpaths. This preventspackaging regressions from surfacing only when a release is attempted.
Closes #411.
Review walkthrough
Validation
cargo publish --dry-run --allow-dirty -p netsuke: passedmake check-fmt: passedmake typecheck: passedmake test: passedmake lint: passedmake markdownlint: passedmake nixie: passedcoderabbit review --agent: completed without a findingReferences