Repository navigation
Report as you go - #59
Merged
Merged
Conversation
- Move Variables and the placeholder substitution into variables.rs - Take the URL in DocAssert::new instead of with_url, dropping Error::NoUrl - Return Option from Variables::from_json, dropping Error::VariablesNotAnObject - Fold Run::run_to_end into DocAssert::run - Move the public API tests to tests/api.rs, the render test to report.rs - Drop the crate-wide while_let_on_iterator allow
VashingMachine
approved these changes
Oct 6, 2026
VashingMachine
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed all 14 changed files against main, at commit 21ecb4c8aa92d5632d3c72f20f725f4a179d0368. I found no actionable regressions or changes needed before merging.
Review covered:
- Public API and reports: the
assert/run/startcontracts agree with the implementation and updated examples. All files are parsed before execution, failed cases remain structured results, and incomplete runs cannot report success. The API break is reflected in the 0.2.0 version bump. - Execution and variables: retry limits count attempts, successful attempts stop immediately, and no delay is added after the final failure. Invalid expected JSON and request headers are checked before sending. Extracted variables are committed together only after the response passes, so a failed attempt cannot partially update shared state. Non-ASCII response header mismatches no longer require a fallible conversion followed by
unwrap. - CLI and progress: test identifiers are printed and flushed before awaiting requests, failure details are rendered at the end, and argument/parse errors go to stderr with the documented exit codes. The integration tests exercise output timing against a held response, exact failure output, sequential execution, and variables shared across files.
- Documentation and CI: the examples use the new API, and CI now lints and tests the feature-gated binary. The cancellation behavior of
Run::nextis explicitly documented and tested; I treated it as the declared limitation of this API.
Local validation passed:
cargo test --features binary: 37 unit tests, 11 public API tests, 6 CLI tests, and 17 doctests.cargo fmt --all -- --check.cargo clippy --all-targets --features binary -- -D warnings.cargo doc --no-deps --features binary.git diff --checkfor the PR diff.
GitHub CI is also green, and the PR head still matches the reviewed commit. Approved.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
doc-assert now reports results as they happen, like
cargo testdoes, instead of printing one block when the whole run is over. The library API is reworked around this, so the crate goes out as 0.2.0 (breaking).Reporting as you go
Each test case is named as soon as its request is sent and marked ✅ or ❌ once it is done, so a slow or retried request shows what is being waited for. The details of the failures follow at the end:
The CLI and
DocAssert::assert()print the same output. Inside a test, cargo captures it like any test output: it is shown when the test fails, or as it is printed withcargo test -- --nocapture.All of this is built on
Run:DocAssert::start()parses every documentation file up front, so a parse error is reported before any request is sent and the number of test cases is known in advance.run.next().awaitexecutes one test case and returns itsTestCaseResult.run.print_progress().awaitexecutes the rest, printing the output above.run.finish()returns theReportof what was executed so far.API changes (breaking)
assert()(); prints the progress, panics unless every test case passed#[tokio::test](the common case)run()Result<Report, Error>; prints nothingstart()Result<Run, Error>Erronly means the run could not happen at all: no documentation file was given, or a file could not be parsed. Failed test cases are reported in theReport, andReport::passed()gives the verdict.Migration
DocAssert::new().with_url(url)DocAssert::new(url)assert().awaitreturningResult<Report, AssertionError>assert().await, panicking on failure, orrun().awaitreturningResult<Report, Error>Err(AssertionError::TestSuiteError(report))Ok(report)withreport.passed() == falseErr(AssertionError::ParsingError(String))Err(Error::Parse { doc_path, reason })insert_string,insert_int,insert_float,insert_bool,insert_valueinsert(name, value), which accepts anythingInto<serde_json::Value>insert_null(name)insert(name, Value::Null)Variables::from_json(..) -> Result<Self, String>Variables::from_json(..) -> Option<Self>DocAssert<'a>borrowing&strargumentsDocAsserttaking ownedimpl Into<String>argumentsStructured results
Reporthasresults(),failures()(yielding(&TestCaseId, &Failure)pairs),total_count(),executed_count(),passed_count(),failed_count(),not_run_count(),passed()andsummary().TestCaseResulthasid(),passed()andfailure(). ATestCaseIdholds the method, URI, doc path and line.Failuresays why a test case did not pass:UnresolvedVariables { names }InvalidDocumentation { line_number, reason }RequestFailed { reason }ResponseMismatch { line_number, cause: Mismatch }Mismatchcovers what the server got wrong: status code, header, missing header, body, variable not found, unreadable or malformed body.Error,FailureandMismatchare#[non_exhaustive], so variants can be added without a breaking release.Behaviour changes
; N not run.[ignore]path is now a parse error.InvalidDocumentationbefore any request is sent. These used to be sent and retried as often as the retry policy allowed.[retry]: # (0, …)is rejected as a parse error that names its line. Previously it produced a test case that could never run. The README now says that the first number is the number of attempts, the first one included, which is what the code always did.Error: cannot parse <file>: ….Internal changes
lib.rscontains only the public entry point:DocAssert,Run,Error, and the re-exports. Results and their rendering live inreport.rs;Variablesand placeholder substitution live invariables.rs.FailureandMismatchvalues instead of strings.--features binary, so the binary is linted and tested. Previously it was only built.Known limitation
Run::nextis not cancellation safe.tokio::time::timeoutorselect!, that test case is abandoned.Implementing
StreamforRunwas considered and rejected:Runwould makenextresumable after a drop, but resuming the abandoned request is not what someone applying a timeout wants.while letloop already covers what stream combinators would add.The planned follow-up is a per-request timeout, for example
DocAssert::with_timeout. A timed-out request would then be reported as an ordinaryRequestFailed.Testing
cargo fmt --check,cargo clippy --all-targets --features binary -- -D warningsandcargo docpass with no warnings.cargo test --features binarypasses:tests/api.rs, against the public API only;tests/cli.rs, against the binary;#[cfg(test)].tests/cli.rsruns the binary against a local server that holds an endpoint open. This proves:make sanitypasses againstsample-api.tests/functional/readmes/2_README_in.mdand3_README_in.mdfail onmaintoo, because the expected"My First Blog- UPDATED"is missing a space. They are not changed here.🤖 Generated with Claude Code