Skip to content

Fix/message quality expectations - #292

Merged
fredbi merged 4 commits into
go-openapi:masterfrom
fredbi:fix/message-quality-expectations
Aug 25, 2026
Merged

Fix/message quality expectations#292
fredbi merged 4 commits into
go-openapi:masterfrom
fredbi:fix/message-quality-expectations

Conversation

@fredbi

@fredbi fredbi commented Aug 25, 2026

Copy link
Copy Markdown
Member

Change type

Please select: 🆕 New feature or enhancement|🔧 Bug fix'|📃 Documentation update

Short description

Fixes

Full description

Checklist

  • I have signed all my commits with my name and email (see DCO. This does not require a PGP-signed commit
  • I have rebased and squashed my work, so only one commit remains
  • I have added tests to cover my changes.
  • I have properly enriched go doc comments in code.
  • I have properly documented any breaking change.

fredbi and others added 4 commits August 25, 2026 15:00
Both Test_MessageQuality*_Issue44 passed haltOnErrors=true, while testWalkSpecs
runs every fixture with t.Parallel(). checkMustHalt therefore stopped the run on
whichever fixture lost the race and the rest were never reported, so the fixture
named in the failure changed from run to run.

haltOnErrors now follows DebugTest, which already decides whether the fixtures
run serially. A normal run reports every failing fixture; a run with
SWAGGER_DEBUG_TEST=1 still stops at the first one, serially, which is what that
mode is for.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
expected_messages.yaml pins some 700 messages over 50 fixtures and had no way to
be re-derived, so it drifted: the -enable-long lane has been red since v0.23.0,
growing from 7 failing fixtures to 17.

TestGenExpectations runs every fixture through the validator in both modes and
rewrites the file. It edits the yaml.Node tree rather than re-marshalling the
document, so the 48 comments in the file survive: an expectation that still
matches keeps its node and its spelling, one that matches nothing is deleted, an
actual message nothing covers is appended, and withContinueOnErrors is set from
whether the message also appears in the stop-on-errors run.

It skips unless SWAGGER_GEN is set, and running it twice changes nothing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
The messages now name the parameter they are about - "parameters.0.in" in the
stop-on-errors run and "parameters.myid.in" in the continue-on-errors one, where
the file still expected "parameters.in" - and example validation reports
warnings that did not exist when the file was written. Regenerated with
TestGenExpectations. The two "could not resolve reference" expectations were
widened by hand for the "cannot load spec:" prefix loads now adds, rather than
replaced by a literal carrying an absolute path.

Three fixtures record a defect instead of a message, each with a todo saying so:

- fixture-581.yaml and fixture-additional-items-3.yaml no longer load at all,
  and have not since v0.23.0, so both now expect a load error. The first spells
  "maximum: 18446744073709551615", a YAML integer beyond int64, which the
  conversion refuses; the second spells "properties: {v: string}", which is not
  a schema and spec.Schema refuses. Both were written to exercise validation and
  reach no validator today. What fixture-581.yaml now demonstrates is the
  unmarshalling error itself, which names the offending value but not the field
  it came from.
- fixture-constraints-on-numbers.yaml does not report "param3 in query should be
  a multiple of 10", though the fixture's own comment says it should. MultipleOf
  compares data/factor through conv.IsFloat64AJSONInteger, whose tolerance is
  relative: at a quotient of 2.1e8 it accepts a remainder up to 0.2, and
  2147483648/10 = 214748364.8 falls inside that. MultipleOfInt and MultipleOfUint
  decide the same numbers correctly, so only a value arriving as a float64 is
  affected. The todo says why this is not a tolerance to retune: multipleOf is
  only decidable with decimal arithmetic.

The -enable-long lane is green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
@codecov

codecov Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.92%. Comparing base (d6d7bac) to head (ab4461e).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #292   +/-   ##
=======================================
  Coverage   96.92%   96.92%           
=======================================
  Files          34       34           
  Lines        3771     3771           
=======================================
  Hits         3655     3655           
  Misses        111      111           
  Partials        5        5           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@fredbi
fredbi merged commit 07d44fe into go-openapi:master Aug 25, 2026
25 checks passed
@fredbi
fredbi deleted the fix/message-quality-expectations branch August 25, 2026 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant