Hidden warnings / errors (?) due to `@quietly`

[fingolfin]

Due to confusing CI failures I got in PRs #2349 and #2683 I patched test/examples/make.jl to remove all @quietly statements (I picked that file because it is were the problems I was confused about showed up, but it may a good idea to look at other files. The resulting output shows a bunch of IMHO issues that show certain test cases are not working at all how they should be.

The problem with this is that when a new actual error happens in the affected files, anyone debugging those is confused by a ton of unrelated errors popping up (but it's hard to know they are unrelated).

Perhaps we should use @quietly less? Or perhaps it should not suppress warnings?

Here is the output; I am going to interrupt it for comments at points

     Testing Running tests...
[ Info: Building example/make.jl
[ Info: Building mock package docs: HTMLWriter / deployment build
[ Info: SetupBuildDirectory: setting up build directory.
[ Info: Doctest: skipped.
[ Info: ExpandTemplates: expanding markdown templates.
┌ Warning: Invalid type of object in @eval in examples/builds/html/src/issue491.md

Is this supposed to happen in that particular test (I think it's the one in lines 30-32

Also, the source file really is test/examples/src/issue491.md.

(But perhaps it is being copied by the test mechanism? I did not dig deeper yet. Anyway, observing this made me look and at least resulted in PR #2796)

│ ```@eval
│ "expanded_"*"eval"
│ ```
│ Evaluated to `String`, but should be one of
│  - Nothing
│  - Markdown.MD
│ Falling back to textual code block representation.
│
│ If you are seeing this warning/error after upgrading Documenter and this used to work,
│ please open an issue on the Documenter issue tracker.
└ @ Documenter ~/Projekte/Julia/packages/JuliaDocs/Documenter.jl/src/utilities/utilities.jl:49

There is a bunch of this and similar ones. Intentional, accident, ...?

┌ Warning: failed to evaluate `@eval` block in examples/builds/html/man/src/man/tutorial.md
│ ```@eval
│ import Markdown
│ code = string(sprint(Base.banner), "julia>")
│ Markdown.MD([Markdown.Code(code)])
│ ```
│   exception =
│    UndefVarError: `banner` not defined in `Base`
│    Suggestion: check for spelling errors or missing imports.
│    Stacktrace:
│     [1] getproperty(x::Module, f::Symbol)
│       @ Base ./Base_compiler.jl:47
│     [2] top-level scope
│       @ tutorial.md:69
│     [3] eval(m::Module, e::Any)
│       @ Core ./boot.jl:489
│     [4] (::Documenter.var"#55#56"{MarkdownAST.Node{Nothing}, Documenter.Page, Documenter.Document, LineNumberNode, Module, MarkdownAST.CodeBlock})()
│       @ Documenter ~/Projekte/Julia/packages/JuliaDocs/Documenter.jl/src/expander_pipeline.jl:712
└ @ Documenter ~/Projekte/Julia/packages/JuliaDocs/Documenter.jl/src/utilities/utilities.jl:49

The Base.banner function is gone in Julia 1.11 and later.

So this test is now not testing what it was supposed to test.

But why is this just a warning, not an error?

┌ Warning: failed to evaluate `@eval` block in examples/builds/html/man/src/man/tutorial.md
│ ```@eval
│ import Markdown
│ code = string(sprint(Base.banner), "julia>")
│ Markdown.MD([Markdown.Code(code)])
│ ```
│   exception =
│    UndefVarError: `code` not defined in `Main.EvalBlockSandbox`
│    Suggestion: add an appropriate import or assignment. This global was declared but not assigned.
│    Stacktrace:
│     [1] top-level scope
│       @ tutorial.md:70
│     [2] eval(m::Module, e::Any)
│       @ Core ./boot.jl:489
│     [3] (::Documenter.var"#55#56"{MarkdownAST.Node{Nothing}, Documenter.Page, Documenter.Document, LineNumberNode, Module, MarkdownAST.CodeBlock})()
│       @ Documenter ~/Projekte/Julia/packages/JuliaDocs/Documenter.jl/src/expander_pipeline.jl:712
└ @ Documenter ~/Projekte/Julia/packages/JuliaDocs/Documenter.jl/src/utilities/utilities.jl:49

I don't get this at all

┌ Warning: Invalid type of object in @eval in examples/builds/html/man/src/man/tutorial.md
│ ```@eval
│ rand(20, 20)
│ ```
│ Evaluated to `Matrix{Float64}`, but should be one of
│  - Nothing
│  - Markdown.MD
│ Falling back to textual code block representation.
│
│ If you are seeing this warning/error after upgrading Documenter and this used to work,
│ please open an issue on the Documenter issue tracker.
└ @ Documenter ~/Projekte/Julia/packages/JuliaDocs/Documenter.jl/src/utilities/utilities.jl:49

What's going on with this test failure? Intentional, accident...?

[ Info: CrossReferences: building cross-references.
┌ Warning: Cannot resolve @ref for md"[`unsafe_convert`](@ref Base.unsafe_convert)" in examples/src/lib/functions.md.
│ - No docstring found in doc for binding `Base.unsafe_convert`.
│ - No docstring found in doc for binding `Base.unsafe_convert`.
└ @ Documenter ~/Projekte/Julia/packages/JuliaDocs/Documenter.jl/src/utilities/utilities.jl:49
┌ Warning: Cannot resolve @ref for md"[`cconvert`](@ref Base.cconvert)" in examples/src/lib/functions.md.
│ - No docstring found in doc for binding `Base.cconvert`.
│ - No docstring found in doc for binding `Base.cconvert`.
└ @ Documenter ~/Projekte/Julia/packages/JuliaDocs/Documenter.jl/src/utilities/utilities.jl:49

There are a ton more of these warnings which I am dropping here to not exceed GitHub's limits.

┌ Warning: invalid local link/image: file does not exist in examples/src/index.md
│   link =
│    @ast MarkdownAST.Link("some:path", "") do
│      MarkdownAST.Text("Colons not allowed on Windows ")
│      MarkdownAST.Text("–")
│      MarkdownAST.Text(" ")
│      MarkdownAST.Code("some:path")
│    end
│
└ @ Documenter ~/Projekte/Julia/packages/JuliaDocs/Documenter.jl/src/utilities/utilities.jl:49
┌ Warning: invalid local link/image: file does not exist in examples/src/index.md
│   link =
│    @ast MarkdownAST.Link(":path", "") do
│      MarkdownAST.Text("No \"drive\" ")
│      MarkdownAST.Text("–")
│      MarkdownAST.Text(" ")
│      MarkdownAST.Code(":path")
│    end
│
└ @ Documenter ~/Projekte/Julia/packages/JuliaDocs/Documenter.jl/src/utilities/utilities.jl:49
┌ Warning: invalid local link/image: file does not exist in examples/src/index.md
│   link =
│    @ast MarkdownAST.Link("X:\\some\\path", "") do
│      MarkdownAST.Text("Absolute Windows paths ")
│      MarkdownAST.Text("–")
│      MarkdownAST.Text(" ")
│      MarkdownAST.Code("X:\\some\\path")
│    end
│
└ @ Documenter ~/Projekte/Julia/packages/JuliaDocs/Documenter.jl/src/utilities/utilities.jl:49

Are those warning part of the test? If not, can we suppress them? If yes, how
can we make sure these warnings are "OK", but still are informed about other unintended warnings?

[ Info: CheckDocument: running document checks.
┌ Warning: no footnotes found for 'footnote' in examples/src/lib/functions.md.
└ @ Documenter ~/Projekte/Julia/packages/JuliaDocs/Documenter.jl/src/utilities/utilities.jl:49

???

[ Info: Populate: populating indices.
[ Info: RenderDocument: rendering document.
[ Info: HTMLWriter: rendering HTML pages.
┌ Warning: Generated HTML over size_threshold_warn limit: example-output.md
│     Generated file size: 123.63 (KiB)
│     size_threshold_warn: 100.0 (KiB)
│     size_threshold:      200.0 (KiB)
│     HTML file:           example-output/index.html
└ @ Documenter.HTMLWriter ~/Projekte/Julia/packages/JuliaDocs/Documenter.jl/src/html/HTMLWriter.jl:1996

Lots of these warnings about size thresholds being exceeded are shown. Surely they
are not all there to test this feature? Perhaps some .md files should be split up,
or that threshold be increased for the test?

┌ Warning:     Unexpected Julia interpolation in the Markdown. This probably means that you have an
│     unbalanced or un-escaped $ in the text.
│
│     To write the dollar sign, escape it with `\$`
│
│     We don't have the file or line number available, but we got given the value:
│
│     `foo` which is of type `Symbol`
└ @ Documenter.HTMLWriter ~/Projekte/Julia/packages/JuliaDocs/Documenter.jl/src/html/HTMLWriter.jl:2454
┌ Warning:     Unexpected Julia interpolation in the Markdown. This probably means that you have an
│     unbalanced or un-escaped $ in the text.
│
│     To write the dollar sign, escape it with `\$`
│
│     We don't have the file or line number available, but we got given the value:
│
│     `[1 2 3; 4 5 6]` which is of type `Expr`
└ @ Documenter.HTMLWriter ~/Projekte/Julia/packages/JuliaDocs/Documenter.jl/src/html/HTMLWriter.jl:2454

No idea where this is coming from, and thus no idea if it is "intentional".

Presumably PR #2793 will help narrowing this down.

┌ Warning: For 2 @example blocks, the 'text/html' representation of the resulting
│ object is above the threshold (example_size_threshold: 8192 bytes).
│ - 1 blocks had no image MIME show() method representation as an alternative.
│   Sticking to the 'text/html' representation (largest block is 113087 bytes).
│   On pages: example-output.md
│ - 1 blocks had 'image/svg+xml' fallback image representation available, using that.
│   On pages: example-output.md
└ @ Documenter.HTMLWriter ~/Projekte/Julia/packages/JuliaDocs/Documenter.jl/src/html/HTMLWriter.jl:908

Ditto: where is this from? is it part of the test or not?

There's more, but a lot seems to be repetitions of the issues mentioned above, so I assume if we can resolve them, it will fix many of the repetitions.

[mortenpi]

I didn't dive into the specific cases, but generally:

1. I still think that we should use @quietly as much as possible -- otherwise the logs are impossibly noisy, and it's very hard to separate real errors from the expected (error) output. And there are a lot of error and warning prints that are expected to be printed.
2. The code being suppressed with @quietly should be written in a way that it throws (either due to an error, or due to a failing @test).

I think what's going on here is that in some cases it (no longer?) does (2). The test suite has grown somewhat organically, and I am not surprised if we have developed some problems over time (that are, as you point out, hidden by the @quietly).

I don't really see any other good option here other than to start going through the test suite, to audit if all the test cases are doing what they should be doing, and fix up the ones that are weird/wrong/broken.

[fingolfin]

Yeah, I am not advocating for fully removing @quietly, just that it causes a problem here because it obscures issues; I'd hope we can find a balance.

I just realized that the code is using warnonly = true quite a bit, a good start should be to change that to false. And if there are tests which truly must show a warning that we need to ignore, then perhaps those can be isolated, to minimize the impact...

Oh and the new treat_markdown_warnings_as_error should be set to true I guess

[fingolfin]

PR #2803 is a consequence of looking into this issue, as together with PR #2796 it helped me track down their sources (those two PRs conflict; whichever is merged first, I then need to update the other)

I also will make a PR shortly that resolves a bunch of warnings.

But I also would like to have a setup dedicated to testing warnings, both to get code coverage, and also in order to be able to systematically test locrepr. Not sure how to best do that: I guess it would require running makedoc without quietly, but still capturing the output, or at least part of it, and comparing that suitably...

[mortenpi]

A lot of the error cases should be contained here FWIW, though I am not saying that this is by any means the only place where we test error conditions: https://github.com/JuliaDocs/Documenter.jl/tree/master/test/errors

I just realized that the code is using warnonly = true quite a bit, a good start should be to change that to false.

Oh and the new treat_markdown_warnings_as_error should be set to true I guess

Yes to both in my opinion.

Not sure how to best do that: I guess it would require running makedoc without quietly, but still capturing the output, or at least part of it, and comparing that suitably...

You can always have IOCapture.capture block around any code you want for this. I think we do that in the doctest tests.