Conversation
|
Warning Rate limit exceeded@leynos has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 38 seconds before requesting another review. ⌛ 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. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
Summary by CodeRabbit
WalkthroughAdd new dependencies Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Function
participant MITM as start_mitm()
participant Server as Hyper Server
participant Handler as Request Handler
Test->>MITM: Call start_mitm()
MITM->>Server: Start server on local port
Server->>Handler: On request, call handler closure
Handler-->>Server: Return response
Server-->>Test: Serve responses for test duration
Test->>MITM: Abort server task after test
sequenceDiagram
participant GraphQLClient
participant Env as Environment
participant serde as serde_path_to_error
participant User as User
User->>GraphQLClient: new(token, transcript)
GraphQLClient->>Env: Check GITHUB_GRAPHQL_URL
Env-->>GraphQLClient: Return endpoint or default
User->>GraphQLClient: run_query(query, vars)
GraphQLClient->>serde: Deserialize response with path tracking
serde-->>GraphQLClient: Error with path if failure
GraphQLClient-->>User: Error message with path and snippet
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Reviewer's GuideIntegrate serde_path_to_error for detailed GraphQL deserialization diagnostics, add support for custom GraphQL endpoints via GITHUB_GRAPHQL_URL, and cover the new behavior with both unit and end-to-end tests. Sequence diagram for GraphQLClient deserialization error reportingsequenceDiagram
participant Client as GraphQLClient
participant Server as GraphQL API
participant Serde as serde_path_to_error
Client->>Server: Send GraphQL query
Server-->>Client: Return JSON response
Client->>Serde: Attempt to deserialize response
alt Deserialization fails
Serde-->>Client: Return error with path
Client-->>Client: Format error with path and snippet
Client-->>Caller: Return VkError::BadResponseSerde
else Deserialization succeeds
Serde-->>Client: Return deserialized object
Client-->>Caller: Return object
end
Class diagram for improved GraphQLClient error diagnosticsclassDiagram
class GraphQLClient {
+new(token: &str, transcript: Option<PathBuf>) Result<Self, io::Error>
+with_endpoint(token: &str, endpoint: &str, transcript: Option<PathBuf>) Result<Self, io::Error>
+fetch_and_deserialize<T>(...) Result<T, VkError>
}
class VkError {
BadResponse(String)
BadResponseSerde(String)
}
GraphQLClient --> VkError : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes - here's some feedback:
- Extract the snippet truncation logic (including the magic
200length) into a helper or constant to improve readability and maintainability. - The two tests for missing
reviewThreads(one in the main tests module and one intests/e2e_missing_nodes.rs) are redundant—consider consolidating them into a single, clear end-to-end test. - The spawned HTTP server in the e2e test never shuts down, which can cause the test to hang—add a shutdown signal or a timeout to ensure the server is torn down after the assertion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Extract the snippet truncation logic (including the magic `200` length) into a helper or constant to improve readability and maintainability.
- The two tests for missing `reviewThreads` (one in the main tests module and one in `tests/e2e_missing_nodes.rs`) are redundant—consider consolidating them into a single, clear end-to-end test.
- The spawned HTTP server in the e2e test never shuts down, which can cause the test to hang—add a shutdown signal or a timeout to ensure the server is torn down after the assertion.
## Individual Comments
### Comment 1
<location> `tests/e2e_missing_nodes.rs:15` </location>
<code_context>
+
+type Handler = Arc<Mutex<Box<dyn FnMut(&Request<Body>) -> Response<Body> + Send>>>;
+
+fn generate_ca() -> CertificateAuthority {
+ use std::process::Command as PCommand;
+ let dir = tempfile::tempdir().expect("tempdir");
</code_context>
<issue_to_address>
Consider extracting the CA/proxy setup into a reusable helper or switching to a mock server crate to simplify and clarify your test code.
Here’s a way to both slim down that nested setup *and* keep your MITM-based tests if you need them, by extracting all of the CA/proxy wiring into a small helper, and then letting each test only declare its stub and its assertions. You can also toss in a simpler `wiremock` example if you’re open to a new dev-dependency.
1) Extract your helper into e.g. `tests/utils.rs`:
```rust
// tests/utils.rs
use std::{net::SocketAddr, sync::{Arc, Mutex}};
use third_wheel::{
CertificateAuthority, MitmProxy, ThirdWheel,
hyper::{Body, Request, Response, StatusCode},
mitm_layer,
};
pub type Handler = Arc<Mutex<Box<dyn FnMut(&Request<Body>) -> Response<Body> + Send>>>;
pub fn start_mitm() -> (SocketAddr, Handler) {
let handler: Handler = Arc::new(Mutex::new(Box::new(|_req| {
Response::builder()
.status(StatusCode::NOT_FOUND)
.body(Body::from("No handler"))
.unwrap()
})));
let handler_clone = handler.clone();
// generate_ca() can live here, or inline it if you use it only once
let ca = {
let dir = tempfile::tempdir().unwrap();
let cert = dir.path().join("cert.pem");
let key = dir.path().join("key.pem");
std::process::Command::new("openssl")
.args(&[
"req","-x509","-newkey","rsa:4096",
"-keyout", key.to_str().unwrap(),
"-out", cert.to_str().unwrap(),
"-days","1","-passout","pass:third-wheel",
"-subj","/C=US/ST=test/L=test/O=vk/CN=vk.test",
])
.status().unwrap();
CertificateAuthority::load_from_pem_files_with_passphrase_on_key(
cert, key, "third-wheel"
).unwrap()
};
let layer = mitm_layer(move |req: Request<Body>, _tw: ThirdWheel| {
let resp = (handler_clone.lock().unwrap())(&req);
Box::pin(async move { Ok(resp) })
});
let proxy = MitmProxy::builder(layer, ca).build();
let (addr, fut) = proxy.bind("127.0.0.1:0".parse().unwrap());
tokio::spawn(fut);
(addr, handler)
}
```
2) Then your test in `tests/e2e.rs` becomes super‐focused:
```rust
use assert_cmd::Command;
use predicates::str::contains;
use serde_json::json;
use tests::utils::start_mitm;
#[tokio::test]
async fn e2e_missing_nodes_reports_path() {
let (addr, handler) = start_mitm();
// just swap in your JSON stub
*handler.lock().unwrap() = Box::new(move |_req| {
let body = json!({
"data": {
"repository": {
"pullRequest": {
"reviewThreads": {
"pageInfo": {
"hasNextPage": false,
"endCursor": null
}
}
}
}
}
})
.to_string();
Response::builder()
.status(200)
.header("Content-Type", "application/json")
.body(Body::from(body))
.unwrap()
});
Command::cargo_bin("vk").unwrap()
.env("GITHUB_GRAPHQL_URL", format!("http://{}", addr))
.env("GITHUB_TOKEN", "dummy")
.args(&["pr", "https://github.com/leynos/cmd-mox/pull/25"])
.assert()
.failure()
.stderr(contains("repository.pullRequest.reviewThreads"))
.stderr(contains("snippet:"));
}
```
— or, if you’re OK pulling in a mock‐server crate, you can drop *all* of that MITM/CA logic and instead do this with `wiremock` (or `httpmock`):
```rust
use wiremock::{MockServer, Mock, ResponseTemplate};
use wiremock::matchers::{method, any};
use serde_json::json;
#[tokio::test]
async fn e2e_missing_nodes_reports_path() {
let server = MockServer::start().await;
let body = json!({
"data": { /* … */ }
}).to_string();
Mock::given(any())
.respond_with(
ResponseTemplate::new(200)
.set_header("Content-Type", "application/json")
.set_body_string(body),
)
.mount(&server)
.await;
Command::cargo_bin("vk").unwrap()
.env("GITHUB_GRAPHQL_URL", server.uri())
.env("GITHUB_TOKEN", "dummy")
.args(&["pr", "https://github.com/leynos/cmd-mox/pull/25"])
.assert()
.failure()
.stderr(contains("repository.pullRequest.reviewThreads"))
.stderr(contains("snippet:"));
}
```
Both approaches keep your test intent crystal‐clear and remove deep nesting in each test.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
Cargo.toml(1 hunks)src/main.rs(3 hunks)tests/e2e_missing_nodes.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
Cargo.toml
📄 CodeRabbit Inference Engine (AGENTS.md)
Cargo.toml: Use explicit version ranges inCargo.tomland keep dependencies up-to-date.
Mandate caret requirements for all dependencies: All crate versions specified inCargo.tomlmust use SemVer-compatible caret requirements (e.g.,some-crate = "1.2.3").
Prohibit unstable version specifiers: The use of wildcard (*), or open-ended inequality (>=) version requirements are strictly forbidden inCargo.toml. Tilde requirements (~) should only be used where a dependency must be locked to patch-level updates for a specific, documented reason.
Files:
Cargo.toml
**/*.rs
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.rs: Every module must begin with a module level (//!) comment explaining the module's purpose and utility.
Document public APIs using Rustdoc comments (///) so documentation can be generated with cargo doc.
Place function attributes after doc comments.
Do not usereturnin single-line functions.
Use predicate functions for conditional criteria with more than two branches.
Prefer immutable data and avoid unnecessarymutbindings.
Handle errors with theResulttype instead of panicking where feasible.
Prefer semantic error enums: Derivestd::error::Error(via thethiserrorcrate) for any condition the caller might inspect, retry, or map to an HTTP status.
Use an opaque error only at the app boundary: Useeyre::Reportfor human-readable logs; these should not be exposed in public APIs.
Never export the opaque type from a library: Convert to domain enums at API boundaries, and toeyreonly in the mainmain()entrypoint or top-level async task.
Clippy warnings MUST be disallowed.
Fix any warnings emitted during tests in the code itself rather than silencing them.
Where a function is too long, extract meaningfully named helper functions adhering to separation of concerns and CQRS.
Where a function has too many parameters, group related parameters in meaningfully named structs.
Where a function is returning a large error consider usingArcto reduce the amount of data returned.
Write unit and behavioural tests for new functionality. Run both before and after making any change.
Avoidunsafecode unless absolutely necessary and document any usage clearly.
Lints must not be silenced except as a last resort.
Lint rule suppressions must be tightly scoped and include a clear reason.
Preferexpectoverallow.
Prefer.expect()over.unwrap().
Useconcat!()to combine long string literals rather than escaping newlines with a backslash.
Files:
tests/e2e_missing_nodes.rssrc/main.rs
⚙️ CodeRabbit Configuration File
**/*.rs: * Seek to keep the cyclomatic complexity of functions no more than 12.
Adhere to single responsibility and CQRS
Place function attributes after doc comments.
Do not use
returnin single-line functions.Move conditionals with >2 branches into a predicate function.
Avoid
unsafeunless absolutely necessary.Every module must begin with a
//!doc comment that explains the module's purpose and utility.Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.Use
rstestfixtures for shared setup and to avoid repetition between tests.Replace duplicated tests with
#[rstest(...)]parameterised cases.Prefer
mockallfor mocks/stubs.Prefer
.expect()over.unwrap()Ensure that any API or behavioural changes are reflected in the documentation in
docs/Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/Files must not exceed 400 lines in length
- Large modules must be decomposed
- Long match statements or dispatch tables should be decomposed by domain and collocated with targets
- Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.
Files:
tests/e2e_missing_nodes.rssrc/main.rs
🧬 Code Graph Analysis (1)
tests/e2e_missing_nodes.rs (1)
src/main.rs (1)
new(143-147)
🔇 Additional comments (5)
Cargo.toml (1)
12-12: LGTM on dependency addition.The
serde_path_to_errordependency follows the required caret requirement format and directly supports the enhanced error diagnostics functionality.tests/e2e_missing_nodes.rs (2)
44-62: LGTM on mock server implementation.The handler setup with Arc<Mutex<Box>> allows for runtime handler replacement which is perfect for the test's needs. The server binding and spawning is handled correctly.
64-99: Verify test assertions cover the enhanced error reporting.The test correctly sets up a mock response missing the
nodesfield and verifies both the JSON path and snippet are present in the error output, which aligns perfectly with the PR objectives.The test effectively validates the enhanced error diagnostics by checking for both
repository.pullRequest.reviewThreadspath andsnippet:in the stderr output.src/main.rs (2)
144-146: LGTM on environment variable support.The implementation correctly reads
GITHUB_GRAPHQL_URLand falls back to the constant, enabling custom endpoints for testing as demonstrated in the e2e test.
226-240: Excellent enhancement to error diagnostics.The switch from direct
serde_json::from_valuetoserde_path_to_error::deserializeprovides significantly better error context. The error message format includes the inner error, JSON path, and a truncated snippet, which will greatly improve debugging experience.The 200-character truncation limit prevents excessively long error messages whilst still providing useful context.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
Cargo.toml(2 hunks)src/main.rs(4 hunks)tests/e2e.rs(3 hunks)tests/utils.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
Cargo.toml
📄 CodeRabbit Inference Engine (AGENTS.md)
Cargo.toml: Use explicit version ranges inCargo.tomland keep dependencies up-to-date.
Mandate caret requirements for all dependencies: All crate versions specified inCargo.tomlmust use SemVer-compatible caret requirements (e.g.,some-crate = "1.2.3").
Prohibit unstable version specifiers: The use of wildcard (*), or open-ended inequality (>=) version requirements are strictly forbidden inCargo.toml. Tilde requirements (~) should only be used where a dependency must be locked to patch-level updates for a specific, documented reason.
Files:
Cargo.toml
**/*.rs
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.rs: Every module must begin with a module level (//!) comment explaining the module's purpose and utility.
Document public APIs using Rustdoc comments (///) so documentation can be generated with cargo doc.
Place function attributes after doc comments.
Do not usereturnin single-line functions.
Use predicate functions for conditional criteria with more than two branches.
Prefer immutable data and avoid unnecessarymutbindings.
Handle errors with theResulttype instead of panicking where feasible.
Prefer semantic error enums: Derivestd::error::Error(via thethiserrorcrate) for any condition the caller might inspect, retry, or map to an HTTP status.
Use an opaque error only at the app boundary: Useeyre::Reportfor human-readable logs; these should not be exposed in public APIs.
Never export the opaque type from a library: Convert to domain enums at API boundaries, and toeyreonly in the mainmain()entrypoint or top-level async task.
Clippy warnings MUST be disallowed.
Fix any warnings emitted during tests in the code itself rather than silencing them.
Where a function is too long, extract meaningfully named helper functions adhering to separation of concerns and CQRS.
Where a function has too many parameters, group related parameters in meaningfully named structs.
Where a function is returning a large error consider usingArcto reduce the amount of data returned.
Write unit and behavioural tests for new functionality. Run both before and after making any change.
Avoidunsafecode unless absolutely necessary and document any usage clearly.
Lints must not be silenced except as a last resort.
Lint rule suppressions must be tightly scoped and include a clear reason.
Preferexpectoverallow.
Prefer.expect()over.unwrap().
Useconcat!()to combine long string literals rather than escaping newlines with a backslash.
Files:
tests/utils.rssrc/main.rstests/e2e.rs
⚙️ CodeRabbit Configuration File
**/*.rs: * Seek to keep the cyclomatic complexity of functions no more than 12.
Adhere to single responsibility and CQRS
Place function attributes after doc comments.
Do not use
returnin single-line functions.Move conditionals with >2 branches into a predicate function.
Avoid
unsafeunless absolutely necessary.Every module must begin with a
//!doc comment that explains the module's purpose and utility.Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.Use
rstestfixtures for shared setup and to avoid repetition between tests.Replace duplicated tests with
#[rstest(...)]parameterised cases.Prefer
mockallfor mocks/stubs.Prefer
.expect()over.unwrap()Ensure that any API or behavioural changes are reflected in the documentation in
docs/Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/Files must not exceed 400 lines in length
- Large modules must be decomposed
- Long match statements or dispatch tables should be decomposed by domain and collocated with targets
- Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.
Files:
tests/utils.rssrc/main.rstests/e2e.rs
🔇 Additional comments (8)
Cargo.toml (1)
12-12: Remove verification comment for serde_path_to_errorserde_path_to_error = "0.1" resolves to v0.1.17 (released 2025-03-03) with no security advisories and meets the SemVer caret requirement. No action required.
src/main.rs (4)
115-126: LGTM!The constants and snippet function are well-implemented. The function correctly handles Unicode character boundaries and provides consistent truncation with ellipsis indication.
157-159: LGTM!Reading the endpoint from an environment variable provides the flexibility needed for testing whilst maintaining a sensible default.
222-224: LGTM!Consistent use of the snippet function improves error message formatting.
234-247: Excellent enhancement to error diagnostics.The integration of
serde_path_to_errorsignificantly improves debugging by providing the exact JSON path where deserialization fails, along with a contextual snippet. The error message format is clear and informative.tests/e2e.rs (3)
4-7: LGTM!The import reorganisation and introduction of the utils module improves code organisation and reusability.
22-45: Good refactoring with proper cleanup.The migration to
start_mitm()improves code reuse and the explicithandle.abort()ensures proper resource cleanup after the test.
46-87: Well-implemented test with proper timeout and cleanup.The new test effectively validates the enhanced error reporting functionality. The timeout prevents hanging issues mentioned in the PR description, and the assertions correctly verify both the JSON path and snippet presence in error output.
The test addresses the concerns raised in the past review comment by using proper timeout handling and more specific error assertions that check for both the JSON path and snippet content.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
docs/vk-design.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
docs/**/*.md
📄 CodeRabbit Inference Engine (AGENTS.md)
docs/**/*.md: Use the markdown files within thedocs/directory as a knowledge base and source of truth for project requirements, dependency choices, and architectural decisions.
Proactively update the relevant file(s) in thedocs/directory to reflect the latest state when new decisions are made, requirements change, libraries are added/removed, or architectural patterns evolve.
Files:
docs/vk-design.md
**/*.md
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.md: Documentation must use en-GB-oxendict spelling and grammar, except for the naming of the "LICENSE" file.
Validate Markdown files usingmake markdownlint.
Runmake fmtafter any documentation changes to format all Markdown files and fix table markup.
Validate Mermaid diagrams in Markdown files by runningmake nixie.
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks in Markdown must be wrapped at 120 columns.
Tables and headings in Markdown must not be wrapped.
Use dashes (-) for list bullets in Markdown.
Use GitHub-flavoured Markdown footnotes ([^1]) for references and footnotes.
Files:
docs/vk-design.md
⚙️ CodeRabbit Configuration File
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -our) spelling and grammar
- Paragraphs and bullets must be wrapped to 80 columns, except where a long URL would prevent this (in which case, silence MD013 for that line)
- Code blocks should be wrapped to 120 columns.
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
docs/vk-design.md
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
tests/e2e.rs (1)
31-50: Good refactoring to use the shared MITM utility.The test properly uses the new utility and ensures cleanup. For consistency with Rust HTTP libraries, use
StatusCode::OKinstead of the literal200.- .status(200) + .status(hyper::StatusCode::OK)
♻️ Duplicate comments (3)
Cargo.toml (1)
48-51: Well done on updating hyper to version 1.6.0.The dependency updates properly address the previous review feedback and include the necessary supporting crates for the test infrastructure.
tests/utils.rs (2)
1-17: Module setup follows all guidelines.The documentation clearly describes the module's purpose, and all imports are appropriately used.
16-31: Clean implementation of handler type and graceful shutdown.The thread-safe handler type and shutdown mechanism properly address the previous review feedback about resource cleanup.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
Cargo.toml(2 hunks)tests/e2e.rs(3 hunks)tests/utils.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
⚙️ CodeRabbit Configuration File
**/*.rs: * Seek to keep the cyclomatic complexity of functions no more than 12.
Adhere to single responsibility and CQRS
Place function attributes after doc comments.
Do not use
returnin single-line functions.Move conditionals with >2 branches into a predicate function.
Avoid
unsafeunless absolutely necessary.Every module must begin with a
//!doc comment that explains the module's purpose and utility.Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.Use
rstestfixtures for shared setup and to avoid repetition between tests.Replace duplicated tests with
#[rstest(...)]parameterised cases.Prefer
mockallfor mocks/stubs.Prefer
.expect()over.unwrap()Ensure that any API or behavioural changes are reflected in the documentation in
docs/Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/Files must not exceed 400 lines in length
- Large modules must be decomposed
- Long match statements or dispatch tables should be decomposed by domain and collocated with targets
- Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.
Files:
tests/e2e.rstests/utils.rs
🧬 Code Graph Analysis (1)
tests/e2e.rs (2)
tests/utils.rs (2)
start_mitm(46-89)shutdown(27-30)src/main.rs (1)
new(156-160)
🔇 Additional comments (3)
tests/e2e.rs (2)
1-6: Module documentation properly added.The documentation clearly explains the module's purpose and follows the required format.
52-93: Excellent test for GraphQL error path reporting.The test effectively verifies the enhanced error diagnostics, uses appropriate timeout handling, and ensures proper cleanup. The mock response correctly simulates missing nodes to trigger the error path.
tests/utils.rs (1)
67-76: Service implementation handles requests correctly.The mutex locking with
expect()is appropriate for test code, and ignoring connection errors prevents test flakiness from transient network issues.
leynos
force-pushed
the
codex/find-cause-of-badresponseserde-error
branch
from
8857b4f to
830c945
Compare
1 participant
Summary
serde_path_to_errorfor better error contextGITHUB_GRAPHQL_URLfor custom API endpointsBadResponseSerdereviewThreadsnodesTesting
make fmtmake lintmake test(fails: teste2e_missing_nodes_reports_pathhangs)https://chatgpt.com/codex/tasks/task_e_68889a1b166c8322963f277815a98fc5
Summary by Sourcery
Enhance GraphQL error diagnostics by including failing JSON path and snippet, support custom API endpoints via env var, and add tests for deserialization failures
Enhancements:
Build:
Tests: