test: add API surface stability and smoke tests#203
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Rate limit exceeded
⌛ 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. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (9)
📒 Files selected for processing (2)
WalkthroughThis PR introduces a comprehensive API surface test suite by adding configuration to tests/Cargo.toml (new dependencies: uselesskey-token and insta, plus an api-surface feature) and creating tests/api_surface.rs with 611 lines of tests exercising public APIs across multiple cryptographic modules using snapshot-based verification. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/api_surface.rs`:
- Line 1: Add the crate-level attribute to forbid unsafe code by inserting the
directive #![forbid(unsafe_code)] as the very first line of tests/api_surface.rs
(before the existing module doc comment `//! API surface stability and smoke
tests.`) so the test crate disallows unsafe blocks; ensure the attribute is the
top-most item in the file.
- Around line 587-593: The test file uses function-scoped imports (e.g., use
uselesskey_core::{Factory, Seed}; and the various use uselesskey_*::{...} lines)
in one spot but the project guideline requires crate-level imports; change these
function-scoped imports to top-of-file (crate-level) imports and apply the same
transformation consistently to every test in this file (all occurrences matching
the patterns EcdsaFactoryExt/EcdsaSpec, Ed25519FactoryExt/Ed25519Spec,
HmacFactoryExt/HmacSpec, RsaFactoryExt/RsaSpec, TokenFactoryExt/TokenSpec,
X509FactoryExt/X509Spec and uselesskey_core::{Factory, Seed}) so that imports
are consolidated at module scope and no individual test functions retain their
own duplicate use statements.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (9)
Cargo.lockis excluded by!**/*.locktests/snapshots/api_surface__api_shape_snapshots__core_types_shape.snapis excluded by!**/*.snaptests/snapshots/api_surface__api_shape_snapshots__ecdsa_api_shape.snapis excluded by!**/*.snaptests/snapshots/api_surface__api_shape_snapshots__ed25519_api_shape.snapis excluded by!**/*.snaptests/snapshots/api_surface__api_shape_snapshots__hmac_api_shape.snapis excluded by!**/*.snaptests/snapshots/api_surface__api_shape_snapshots__negative_fixtures_shape.snapis excluded by!**/*.snaptests/snapshots/api_surface__api_shape_snapshots__rsa_api_shape.snapis excluded by!**/*.snaptests/snapshots/api_surface__api_shape_snapshots__token_api_shape.snapis excluded by!**/*.snaptests/snapshots/api_surface__api_shape_snapshots__x509_api_shape.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
tests/Cargo.tomltests/api_surface.rs
| @@ -0,0 +1,611 @@ | |||
| //! API surface stability and smoke tests. | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if forbid(unsafe_code) exists at the start of the file
echo "=== Checking for #![forbid(unsafe_code)] ==="
rg -n '^\s*#!\[forbid\(unsafe_code\)\]' tests/api_surface.rs || echo "NOT FOUND"
echo ""
echo "=== First 20 lines of file ==="
head -20 tests/api_surface.rs
echo ""
echo "=== Check for function-local imports around lines 587-593 ==="
sed -n '585,595p' tests/api_surface.rs | cat -nRepository: EffortlessMetrics/uselesskey
Length of output: 1586
Add #![forbid(unsafe_code)] to crate root.
Line 1 must include #![forbid(unsafe_code)] so unsafe blocks cannot be introduced in this test crate, as required by the coding guidelines for all **/*.rs files.
Proposed fix
+#![forbid(unsafe_code)]
//! API surface stability and smoke tests.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| //! API surface stability and smoke tests. | |
| #![forbid(unsafe_code)] | |
| //! API surface stability and smoke tests. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/api_surface.rs` at line 1, Add the crate-level attribute to forbid
unsafe code by inserting the directive #![forbid(unsafe_code)] as the very first
line of tests/api_surface.rs (before the existing module doc comment `//! API
surface stability and smoke tests.`) so the test crate disallows unsafe blocks;
ensure the attribute is the top-most item in the file.
| use uselesskey_core::{Factory, Seed}; | ||
| use uselesskey_ecdsa::{EcdsaFactoryExt, EcdsaSpec}; | ||
| use uselesskey_ed25519::{Ed25519FactoryExt, Ed25519Spec}; | ||
| use uselesskey_hmac::{HmacFactoryExt, HmacSpec}; | ||
| use uselesskey_rsa::{RsaFactoryExt, RsaSpec}; | ||
| use uselesskey_token::{TokenFactoryExt, TokenSpec}; | ||
| use uselesskey_x509::{X509FactoryExt, X509Spec}; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
# First, let's check the file exists and examine lines around 587-593
wc -l tests/api_surface.rsRepository: EffortlessMetrics/uselesskey
Length of output: 96
🏁 Script executed:
# Read the actual content at lines 587-593
sed -n '580,600p' tests/api_surface.rsRepository: EffortlessMetrics/uselesskey
Length of output: 1050
🏁 Script executed:
# Check the broader context of the test function and imports
sed -n '575,610p' tests/api_surface.rsRepository: EffortlessMetrics/uselesskey
Length of output: 1618
🏁 Script executed:
# Search for all function-scoped imports in the file
rg -n '^\s{4,}use\s+uselesskey_' tests/api_surface.rs | head -20Repository: EffortlessMetrics/uselesskey
Length of output: 1188
🏁 Script executed:
# Check if there are module-level imports with cfg guards
rg -n '#\[cfg.*api-surface' tests/api_surface.rs | head -10Repository: EffortlessMetrics/uselesskey
Length of output: 358
Apply crate-level imports consistently across all test functions in this file.
The coding guideline requires crate-level imports in Rust source files. However, note that lines 587-593 follow the same function-scoped import pattern used in every other test function in this file (lines 37, 76, 133, 163, 191, 216, 244, 297, 343, 392). If moving to crate-level is required, apply this change uniformly across all tests in this file for consistency rather than in isolation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/api_surface.rs` around lines 587 - 593, The test file uses
function-scoped imports (e.g., use uselesskey_core::{Factory, Seed}; and the
various use uselesskey_*::{...} lines) in one spot but the project guideline
requires crate-level imports; change these function-scoped imports to
top-of-file (crate-level) imports and apply the same transformation consistently
to every test in this file (all occurrences matching the patterns
EcdsaFactoryExt/EcdsaSpec, Ed25519FactoryExt/Ed25519Spec,
HmacFactoryExt/HmacSpec, RsaFactoryExt/RsaSpec, TokenFactoryExt/TokenSpec,
X509FactoryExt/X509Spec and uselesskey_core::{Factory, Seed}) so that imports
are consolidated at module scope and no individual test functions retain their
own duplicate use statements.
bd3a6d4 to
f80e015
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
f80e015 to
5095aee
Compare
Add 44 API surface stability tests covering all public traits, types, methods, negative fixtures, and insta snapshots for the uselesskey workspace. Tests are gated behind the api-surface feature flag.