fix: the documentation's examples run, and two bugs they exposed - #14
Merged
Conversation
Contributor
|
📚 Docs preview: https://codes.sota-shimozono.com/ExperimentalAPI.jl/previews/PR14/ (updates on each push to this PR) |
The review's own finding, still live: `docs/src/index.md` opened with
@experimental "…" energy(m::Model) = m.β * correction(m)
`Model` and `correction` are defined nowhere. That is the defect the registration review named for
the README — it was fixed there and not here, one file over, where a reader looking at the
documentation site meets it first.
`test/test_readme.jl` covered the README's first block and nothing else. It now executes every
self-contained block under `docs/src` — twenty were shipped unexecuted, and two of them did not
run. Illustrative blocks (`MyPackage`, `Archeion`) and transcripts (`julia>`, `pkg>`) are skipped,
and a control asserts at least five blocks survive that filter, so the loop cannot quietly skip
everything.
`declaring.md` quoted an error message the macro does not produce: it showed
`` `Base.sum(x::Int)` ``, and `_signame` never sees the argument types — the real message says
`` `Base.sum` ``. Corrected.
`doctest = true` in `makedocs`, with one real `jldoctest` in `observing.md`. Only one, and the
page says why: `Entry` prints its module, and Documenter's sandbox does not print as `Main`, so a
doctest of the displayed form would show a line no reader sees at their own REPL. The fields it is
read for are module-independent, so those are the doctest. Verified by changing one character of
the expected output and watching the docs build fail.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Closes #16, #17, #18, #19. Running every self-contained block under `docs/src` turned up two defects in shipped code, not just in prose. `record` swallowed the caller's exception (#16). On the default path it called `Base.rethrow(err)` after the `try`/`catch`, which is legal only inside a `catch`; outside one it raises its own `ErrorException` and the original is lost. The existing assertion — `@test_throws ErrorException` — passed the whole time, because the wrong exception is an `ErrorException` too. The new test pins the message and controls against the old one. `stamp(path) do … end` had no method (#17). `do` puts the function first; the methods took the path first. `record` already takes the function first, so the two verbs disagreed as well. The documentation called public-but-unexported names unqualified in 29 places (#18). Only `@experimental` is exported, so `record(...)` after `using ExperimentalAPI` is `UndefVarError` — and `api.md` states the qualification convention the pages were breaking. `docs/src/index.md` still opened with the example the registration review said does not run (#19): `energy(m::Model) = m.β * correction(m)`, with neither defined. It was fixed in the README and not here. It is now self-contained AND an `@example` block, so Documenter runs it during the build: breaking it again fails with `failed to run @example block in docs/src/index.md`. `test/test_readme.jl` executes every self-contained docs block; illustrative ones are skipped through an explicit list of the placeholders the documentation asks the reader to supply, so a new placeholder fails the test by name rather than being silently skipped. `doctest = true` in `makedocs`, with one `jldoctest` in `observing.md`. Only one, and the page says why: the error block would carry `LoadError` and an absolute-path stacktrace, and the `Entry` display prints its module, which in Documenter's sandbox is not `Main` — a doctest of it would show a line no reader sees at their own REPL. Suite: 990 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
sotashimozono
force-pushed
the
docs/the-front-page-example-runs
branch
from
September 5, 2026 08:43
6fff34f to
6be4638
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This was referenced Sep 5, 2026
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.
A review before resubmitting. Running every self-contained block under
docs/srcturned up two defects in shipped code, not only in prose.Closes #16, #17, #18, #19.
recordswallowed the caller's exception — #16On the default path:
Base.rethrow(err)is legal only inside acatch. Outside one it raises its ownErrorExceptionand the original is lost.The existing test passed the whole time.
@test_throws ErrorException ExperimentalAPI.record(() -> error("boom"))is satisfied byErrorException("rethrow(exc) not allowed outside a catch block")— it pinned the exception type while the caller was getting a different exception entirely. The new test pins the message, with a control asserting the old one is absent.stamp(path) do … endhad no method — #17dopasses the function first; the methods took the path first, so the form the documentation teaches raisedMethodError.recordalready takes the function first, so the two verbs disagreed as well.The documentation called unexported names unqualified — #18, 29 sites
Only
@experimentalis exported.record,audit,mark,compare,stamp, … arepublicand not exported, so every one of those blocks isUndefVarErrorfor a reader who wroteusing ExperimentalAPI— which is what the examples above them show.api.mdstates the convention the pages were breaking:The front page still had the reviewed defect — #19
docs/src/index.mdopened withenergy(m::Model) = m.β * correction(m), neither defined. That is what the registration review named for the README; it was fixed there (#11) and not here.It is now self-contained and an
@exampleblock, so Documenter runs it during the build:That is the reviewer's suggested guard, on the block that needs it most — stronger than the bespoke executor, because it is the tool's own mechanism.
What now checks the rest
test/test_readme.jlexecutes every self-contained block underdocs/src. Illustrative blocks are skipped through an explicit list of the placeholders the documentation asks the reader to supply, so a block needing a new one fails by name rather than being silently skipped. A control asserts at least five blocks survive the filter, and that the front page is still an@example.doctest = true, with onejldoctestinobserving.md. Only one, and the page says why rather than leaving it implied: the error block would carryLoadErrorand an absolute-path stacktrace, and theEntrydisplay prints its module — in Documenter's sandbox that is notMain, so a doctest of it would show a line no reader ever sees at their own REPL.Not in this change
#20 — two placeholder
tracking =URLs (github.com/org/Pkg.jl/issues/12) return 404, while the package's own tests useexample.invalid. Filed rather than folded in.Suite: 990 passed.
🤖 Generated with Claude Code