Enable doctests (COMPLETE): add benchmark script and docs#467
Conversation
- Enabled doctests in Cargo.toml and added `make test-doc` and `make doctest-benchmark` targets. - Added systematic guide and Wireframe-specific doctest policies. - Fixed and rewrote numerous public doctests for compliance. - Introduced `scripts/doctest-benchmark.sh` for runnable/no_run benchmarking. - Converted private/test-only examples to non-doctest fences for scope enforcement. - Updated docs/execplans/enable-and-resolve-doctests.md to complete and record progress. - Improved example type annotations for better inference and clarity. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
Reviewer's GuideEnables Rust doctests as a quality gate, adds a doctest benchmark script and Makefile targets, and adjusts documentation and Rustdoc examples (including fences and explicit types) to satisfy doctest execution and runnable/no_run ratio policy while recording artifacts and updating project guidance. Sequence diagram for running doctest quality gatessequenceDiagram
actor Developer
participant Makefile
participant Cargo as Cargo_test
participant Rustdoc as Rustdoc_runner
participant Script as DoctestBenchmarkScript
Developer->>Makefile: make test-doc
Makefile->>Cargo: RUSTFLAGS="-D warnings" test --doc --all-features
Cargo->>Rustdoc: run doctests for public API
Rustdoc-->>Cargo: results 179 passed, 0 failed, 4 ignored, 2 compile_fail
Cargo-->>Makefile: exit status 0 or nonzero
Makefile-->>Developer: report doctest success or failure
Developer->>Makefile: make doctest-benchmark
Makefile->>Script: ./scripts/doctest-benchmark.sh
Script->>Script: rg list src/*.rs
Script->>Script: awk classify doctest fences
Script->>Script: compute runnable_pct and no_run_pct
Script-->>Makefile: exit 0 if policy met
Script-->>Developer: printed benchmark summary
Flow diagram for doctest-benchmark.sh policy evaluationflowchart TD
A[start doctest-benchmark.sh] --> B[set MIN_RUNNABLE=70 and MAX_NO_RUN=30]
B --> C[collect rust_files with rg --files src -g *.rs]
C --> D[awk scans rust_files for Rustdoc ``` fences]
D --> D1[if info has no_run flag\nincrement no_run]
D --> D2[if info has ignore or compile_fail\nincrement ignored]
D --> D3[if info empty or starts with rust\nincrement runnable]
D --> D4[else increment non_rust]
D1 --> E[after processing all files]
D2 --> E
D3 --> E
D4 --> E
E[compute eligible = runnable + no_run] --> F{eligible == 0?}
F -->|yes| F1[print error no runnable/no_run doctest blocks] --> Z_fail[exit 1]
F -->|no| G[compute runnable_pct and no_run_pct]
G --> H[print benchmark summary counts and percentages]
H --> I{runnable_pct < MIN_RUNNABLE?}
I -->|yes| I1[print error runnable ratio below threshold] --> Z_fail
I -->|no| J{no_run_pct > MAX_NO_RUN?}
J -->|yes| J1[print error no_run ratio exceeds threshold] --> Z_fail
J -->|no| Z_pass[exit 0]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
WalkthroughEnable crate doctests in Cargo.toml, add Makefile targets and a doctest-benchmark script, update policy and execplan docs to require runnable/no_run ratios, and convert many inline doc examples to runnable Rust or plaintext with explicit type/closure annotations. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Make as Makefile
participant Cargo as Cargo/Rustdoc
participant Script as doctest-benchmark.sh
participant FS as Source Files
participant CI as CI
Dev->>Make: run `make test-doc`
Make->>Cargo: run `RUSTFLAGS="-D warnings" cargo test --doc --all-features`
Cargo->>FS: load crate doc examples
FS-->>Cargo: provide doctest blocks
Cargo->>Dev: report doctest results (pass/fail)
Dev->>Make: run `make doctest-benchmark`
Make->>Script: execute `scripts/doctest-benchmark.sh`
Script->>FS: scan fenced code blocks
Script->>Make: report runnable/no_run/ignored counts and enforce thresholds
Script->>CI: exit non-zero on threshold violation
Make->>CI: propagate success/failure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The doctest benchmark script hard-depends on
rgbeing installed and onsrcas the only search root; consider either checking forrgwith a clear error or falling back tofind, and making the search path configurable (e.g., via an argument or env var) if you ever want to include doctests fromdocs/or other crates. scripts/doctest-benchmark.shhard-codes the runnable/no_runthresholds; it may be more maintainable to read these from environment variables (with the current values as defaults) so that policy changes don’t require editing the script.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The doctest benchmark script hard-depends on `rg` being installed and on `src` as the only search root; consider either checking for `rg` with a clear error or falling back to `find`, and making the search path configurable (e.g., via an argument or env var) if you ever want to include doctests from `docs/` or other crates.
- `scripts/doctest-benchmark.sh` hard-codes the runnable/`no_run` thresholds; it may be more maintainable to read these from environment variables (with the current values as defaults) so that policy changes don’t require editing the script.
## Individual Comments
### Comment 1
<location> `docs/execplans/enable-and-resolve-doctests.md:129-131` </location>
<code_context>
+ `cargo test --doc --all-features` passes (`179 passed`, `0 failed`,
+ `4 ignored` + `2 compile_fail` checks).
+- Runnable-vs-`no_run` benchmark met:
+ runnable 128 (71%), `no_run` 51 (28%), ignored/compile_fail 6, non-rust/text
+ 7.
+- Scope policy enforced for private/test-only helpers by converting their code
</code_context>
<issue_to_address>
**nitpick (typo):** Align capitalization of "Rust" in "non-Rust/text" for consistency with earlier usage.
In the Progress section this appears as `non-Rust/text 7.`, but here it’s `non-rust/text`. Please capitalize “Rust” here as well for consistency.
```suggestion
- Runnable-vs-`no_run` benchmark met:
runnable 128 (71%), `no_run` 51 (28%), ignored/compile_fail 6, non-Rust/text
7.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…enhance doctest benchmark script - Updated fenced code blocks from ```text to ```plaintext across multiple Rust source files and docs for consistency. - Improved the doctest benchmark script to allow environment variable overrides, accept search roots as arguments, and utilize ripgrep if available for better performance. - Refined wording in docs for clarity and consistency, including changing 'text' fences to 'plaintext' in doctests and updating benchmark policy explanations. These changes improve documentation clarity and testing reliability. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
|
@coderabbitai Have the following now been resolved? |
This comment was marked as resolved.
This comment was marked as resolved.
Updated references to non-Rust code fences from `text` to `plaintext` for accuracy and consistency in the enable-and-resolve-doctests.md file. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docs/execplans/enable-and-resolve-doctests.md`:
- Around line 94-96: Update the sentence containing "converted deterministic
state/extractor examples from `no_run` to runnable to restore the target
balance." (in docs/execplans/enable-and-resolve-doctests.md) to remove the
duplicated "to" and correct grammar and spelling to en-GB-oxendict by changing
it to "converted deterministic state/extractor examples from `no_run` to
runnable, restoring the target balance." Ensure the replacement preserves the
inline code formatting around `no_run` and `runnable`.
Tweaked wording in the observation about benchmark results for clarity and conciseness. - Changed description of converting deterministic state/extractor examples from `no_run` to runnable to use a more fluent phrasing. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docs/execplans/enable-and-resolve-doctests.md`:
- Around line 93-96: The observation sentence in
docs/execplans/enable-and-resolve-doctests.md currently has a duplicated "to" —
locate the sentence "converted deterministic state/extractor examples from
`no_run` to runnable to restore the target balance." and replace it with
"converted deterministic state/extractor examples from `no_run` to runnable,
restoring the target balance." to fix the grammar and comply with en-GB-oxendict
spelling/grammar.
Summary
Changes
Test plan
Notes
◳ Generated by DevBoxer ◰
ℹ️ Tag @devboxerhub to ask questions and address PR feedback
📎 Task: https://www.devboxer.com/task/05b7b8a6-295c-47bb-af96-c1521bdbc929
Summary by Sourcery
Enable Rust doctests as a enforced quality gate, add benchmarking for doctest coverage, and align documentation and examples with the new doctest policy.
Enhancements:
no_rundoctests.no_runthresholds.no_runor non-doctest text fences as appropriate.Build:
test-docanddoctest-benchmarkmake targets to integrate doctests into the standard quality gates.Documentation:
no_runratios and quality gate results.Tests:
no_runto runnable and ensuring all doctests pass across all features.