test(ingress): meta-test that every column-writing Buffer method has an _opt twin#139
test(ingress): meta-test that every column-writing Buffer method has an _opt twin#139aaaronme wants to merge 2 commits intoquestdb:mainfrom
_opt twin#139Conversation
…` twin Adds a meta-test that scans `buffer.rs` and asserts every column-writing method (identified by the `N: TryInto<ColumnName<'a>>` bound rather than by name prefix) has a matching `_opt` sibling. Catches the case where a new method like `uuid` or `column_uuid` lands without its `_opt` twin. - Adds `regex` to dev-dependencies
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 23 minutes and 27 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds a compile-time test that uses regex to verify every public column-writing method in the Buffer implementation has a corresponding Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
questdb-rs/src/tests/buffer_opt.rs (1)
157-158: Optional: harden the regexes againstpub fnstrings appearing in doc comments / examples.
pub fnpatterns occurring inside///doc comments or//!module docs inbuffer.rs(e.g., usage examples) will be picked up by both regexes. For the writer regex this only matters if a doc example happens to also containTryInto<ColumnNamebetween the name and the next{; for thepub_fnset it's mostly benign (extra entries can only mask a missing_opt, never cause a spurious failure). It's not exploitable today, but a future doc example like/// pub fn uuid<N>(... where N: TryInto<ColumnName<'a>> ...could quietly hide a real gap.Two cheap mitigations if you want to make this future-proof:
- Anchor on indentation typical for
implitems (e.g.,(?m)^ pub fn (\w+)), which excludes///-prefixed lines.- Or strip line comments before scanning:
let src = src.lines().filter(|l| !l.trim_start().starts_with("//")).collect::<Vec<_>>().join("\n");.Up to you — the current implementation is fine for the code as it stands today.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@questdb-rs/src/tests/buffer_opt.rs` around lines 157 - 158, The current regexes pub_fn and writer can match `pub fn` inside doc comments; to harden, either anchor them to impl-style indentation (e.g., update the patterns used for pub_fn and writer to use multiline anchors like (?m)^\s{4}pub fn (\w+) and (?m)^\s{4}pub fn (\w+)[^{]*TryInto<ColumnName) so doc-comment lines are excluded, or preprocess the source (the src string read from buffer.rs) by stripping line comments/doc comments (filter out lines where l.trim_start().starts_with("///") or l.trim_start().starts_with("//") or starts_with("//!")) before running the existing regexes; apply the chosen change to the code that constructs pub_fn and writer so matches only come from real impl/function definitions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@questdb-rs/src/tests/buffer_opt.rs`:
- Around line 157-158: The current regexes pub_fn and writer can match `pub fn`
inside doc comments; to harden, either anchor them to impl-style indentation
(e.g., update the patterns used for pub_fn and writer to use multiline anchors
like (?m)^\s{4}pub fn (\w+) and (?m)^\s{4}pub fn (\w+)[^{]*TryInto<ColumnName)
so doc-comment lines are excluded, or preprocess the source (the src string read
from buffer.rs) by stripping line comments/doc comments (filter out lines where
l.trim_start().starts_with("///") or l.trim_start().starts_with("//") or
starts_with("//!")) before running the existing regexes; apply the chosen change
to the code that constructs pub_fn and writer so matches only come from real
impl/function definitions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e86b5c33-7ad2-4a94-a44c-60ac36e21945
📒 Files selected for processing (2)
questdb-rs/Cargo.tomlquestdb-rs/src/tests/buffer_opt.rs
Adds `(?m)^\s*` to both regexes so matches must start at indentation, not mid-line. A future `pub fn ...` inside a `///` doc example can no longer be picked up — which would otherwise silently mask a missing `_opt` win if the same example also referenced `TryInto<ColumnName`. Today no doc example triggers this, so behavior is unchanged. The fix is preventative.
|
@jerrinot wdyt? |
1 participant
Summary
Follow-up to #134. Adds a meta-test in
tests/buffer_opt.rsthat scansbuffer.rsand asserts every column-writing method has a matching_optsibling.Column writers are detected by the
N: TryInto<ColumnName<'a>>generic bound rather than by acolumn_*name prefix — sosymboland any future unprefixed method (e.g. a hypotheticaluuid) are also covered. If someone landscolumn_uuidoruuidwithout a_optcompanion, the test fails with the missing names listed.Two short regexes do the work;
[^{]*matches across newlines so it spans multi-linewhereclauses without dotall. Addsregexto dev-dependencies (already pulled in transitively by other crates, so no new top-level dep at build time).Test plan
cargo test --lib buffer_opt— all 6 tests pass, including the newtest_every_column_writing_method_has_opt_variant_optmethod frombuffer.rsand confirm the test fails with that method name in the assertion messageSummary by CodeRabbit
Tests
Chores