Repository navigation
Tell errors from warnings, and fail a run on its errors - #409
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hared piece of the reports Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…one warning reads in the singular, and a kind with no name is called by its number Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Related: None
What you can do now that you could not before
Every fault a run reports is now an error — the API fell over, never answered, or accepted what its own document forbids — or a warning — the API answered something its document does not describe, where the document may be the one out of date. Each kind has a title in plain words, and the summary says under each kind what to look at.
A run ends with
1for errors only. An API whose document leaves out the ways it refuses a request no longer turns a build red.--set checks.failOnWarnings=truemakes warnings fail the run too.Try it yourself
You need only Java and git. The "API" is a stand-in of a few lines, on the HTTP server that comes with Java. It answers every request with
201and an empty JSON list, while its document declares only200: every reply is a warning, and nothing is an error.Real output, from a fresh clone of this branch. Ten warnings are printed whole, then only counted:
On
masterthe same run ends with1, and its fault is printed asF920 A Status Code The Document Does Not Declare.The same run, asked to fail on warnings:
What each kind is, its severity, its title and its advice are in
docs/faults.md, under Errors and warnings.How it works
One rule decides the severity. An error is the API failing at its job: it falls over, never answers, or accepts what its own document forbids. A warning is the API answering something its document does not describe, where the document may be the one out of date.
F100F121F206F312F200F205F920F921Each kind's severity, title and advice line are in one table, in one place. The screen,
report.jsonand the exit code all read that table.On the screen a fault starts with its severity, then its title, then the catalogue's number in brackets.
checks.replyOffSchema.report.jsonwritesseverityandtitlebeside the number in every row that counts faults, andadviceonce for each kind. It also countserrorsandwarningsapart intotals.report.json, for comparing tools.The screen keeps room for errors. It prints at most fifty faults, as before, and now at most ten of those may be warnings. Before, a document that leaves out its refusals filled all fifty places with warnings from the first requests on, and one API's 10,902 replies of 500 never reached the screen.
What changed in the code
restest-coreoracle.Severity(ERROR,WARNING).oracle.FaultReading: one table, keyed by the kind's number, giving each kind RESTest reports its severity, title and advice.FaultCategory,WfcFault,RestestFault,Findingand the stored run are unchanged.CheckSettingsgainsfailOnWarnings, last in the group.ReportSettingsgainswarningsShownOnTheConsole.SettingKeyandSettingsknow both, which makes 99 settings.restest-reportConsoleReport: the new fault block; the allowance per severity, said once each; and the summary in two sections.errors()andwarnings()in place offaults().JsonReport:severityandtitlein every fault row;adviceinfaultsByCategory; four new members intotals.restest-cliExitCode:NO_FAULTS/FAULTS_FOUNDbecomeNO_ERRORS/ERRORS_FOUND, still0and1.of(...)takes errors, warnings and whether warnings fail. Every earlier question keeps its order.0and1changes.RunCommandpasses the setting to the console and the counts toExitCode.Decision records
Documentation
docs/faults.md: a section Errors and warnings, and new columns in the table of kinds. The headings are renamed to the titles, and every link to them is fixed.report.md,settings.md(template regenerated),switches.md("Part of" uses the titles),command-line.md(the rows for0and1, and the exception in What frozen means) andknown-limitations.md.README.md's exit-code paragraph; the docs index;DESIGN.md.Tests
FaultReadingTest.ConsoleReportTest,JsonReportTest,ExitCodeTest,SettingsTest;CheckSwitchesCommandTest: a run whose only faults areF200ends with0, and with1when asked;DocumentedReportTest: the faults page's two tables are held toFaultReading;DocumentedSwitchesTest: each check's title, written as one word, is its switch, and is what the check is "part of".AReplyUnreadIsNotUndeclaredTest: its two runs, whose only faults areF920, end with0, no longer1;WholeRunTestandCheckSwitchesCommandTest: they now look for the new titles.Left for later
F100 HTTP Status 500,<n> faults:). That pull request also owes a line in the release notes.report.json.Evidence
Tests: 32 new test methods, none removed.
./mvnw verifyis green on all nine modules, 4,648 tests (4,594 before). CI: added below once it has run.Architecture tests: two added (the faults page's tables against
FaultReading, and each check's title against its switch); none weakened.Mutation score,
restest-oracles: 97% (294 of 304), as onmaster; its code is unchanged.Per-request overhead: measured on swagger-petstore, restcountries and rest-scs. Three one-minute runs of
masterand of this branch, alternated, with the same seed for each pair, on mains power.masterSmoke run, two containerised APIs: the same two APIs run as the smoke job runs them (10 s, seed 20260914, no key), three runs of
masterand of this branch, alternated.master33,569–36,374 faults; this branch 30,899–36,300, of which 25,525–31,711 errors and 4,589–5,374 warnings.master35,767–41,317 faults; this branch 37,649–41,783, of which 2,557–2,827 errors and 35,092–38,956 warnings.1, on both builds.Idle time: 3.9–4.5% in the smoke runs on both builds; 0.5–1.0% in the one-minute runs.
Artefact: the fifteen APIs of the Web Fuzzing Dataset used for the earlier rules, run locally with this branch, a minute each, seed 42.
1to0, because all 622,908 of its faults are warnings.masterspringcommunity/spring-petclinic-rest:4.0.2, as the manual starts it), 30 seconds:F920,F921,F200), and the run ended with0;--set checks.failOnWarnings=trueended with1, saying so under the warnings.Decisions taken
Decided with the maintainer before any code was written:
F206is an error, because accepting what the document forbids is the API failing at a job its document gives it.F205is a warning. A third level,F206as a "high" warning, was weighed and kept for when the oracles that need it arrive.checks.failOnWarnings.F200is Reply off schema, and the switches page and the faults page's headings use the same words.report.faultsShownOnTheConsole's meaning, and addsreport.warningsShownOnTheConsole.0meant no fault and means no error. Against 2.1.0, a run whose only faults areF200ended with1and ends with0; with--set checks.failOnWarnings=trueit ends with1again. ADR-0015's amendment says so, and so doesdocs/command-line.md.Taken while building it:
0ends "…counted in its summary and in its report." It does not end "…in report.json." because the command-line framework breaks long lines after a full stop and printed "report. json".DocumentedCommandLineTestcaught it.faultsByCategory. It is not repeated under each of up to a thousand findings written out.F205example comes from a run of an API withchecks.undeclaredStatusCodeoff.F200example comes from a pet-clinic run with both undeclared checks off.F312example now comes from kafka-rest-proxy: catwatch'sF312s did not reach the screen in this run.FaultReading.isKnownwas removed in simplification: it was public and used only by tests.Findings not acted on:
checks.failOnWarningsout of "the checks" by name.CheckSwitchesCommandTestdoes it for the run with every check off,DocumentedSwitchesTestfor counting the checks' rows.DocumentedSwitchesTest.kindNumberedlooks a kind up by number next toWfcFault.byCode. SharingDocumentedReportTest's map would mean widening a private helper, for little gain.Open questions
FaultReading, and the exit codes above would not change, since every API with anF200also answered 500.masterAGENTS.mdholddocs/DESIGN.mdwithout explicit approval🤖 Generated with Claude Code