Generalize doctest() API

[mortenpi]

Updates the API. You can now simply call doctest(::Module), and it will doctest both docstrings and manual pages, e.g.:

using Test, Documenter, MyPackage
@testset "MyPackage" begin
    doctest(MyPackage)
end

For more control, one can also use the doctest(source, modules) method, which does the heavy lifting (e.g. to test a set of modules, but no pages, you can call doctest(nothing, [Module1, Module2, ...])).

Finally, it also adds the fix keyword to the methods, to run the doctest fixing machinery. This actually means you can fix doctests pretty easily in the REPL now:

julia> using Documenter, MyPkg
julia> doctest(MyPkg, fix=true)

cc @davidanthoff You don't happen to have any suggestions for packages where we could test this in the wild?

Should also fix the dev/ docs for Documenter, since we're now using the doctest function in runtests.jl.

Close #1051, close #1052

[davidanthoff]

I'll create a branch in Query.jl to test this!

[davidanthoff]

I tried it with queryverse/Query.jl#269, and it all seems to work. The thing won't work for the CI builds because they don't pick up this branch here of Documenter, but locally it all looked great.

One thing I particularly like is that this does not pick up anything from the docs/Project.toml or docs/Manifest.toml, but instead it all uses the env that is specified for the tests in the main package Project.toml. I think that is exactly right!

[fredrikekre]

I think that is exactly right!

Why?

[davidanthoff]

I'm not checking in a docs/Manifest.toml, so I generally feel that shouldn't play a role.

I think one important aspect is that if my active env is say shared foo, and I have deved my package in that env (and potentially other packages as well), then I would want the doc test run to pick up any other packages that I've deved in foo for that test run, in the same way that a test run picks up specific versions of other packages that I have in foo. I'm actually not sure now whether that is happening with the current PR here for the doctests, but generally that is what I'd expect, and that seems somewhat easier if the doctests run in the same env as the general tests? Not sure.

[fredrikekre]

So you would have to add all doc dependendencies as test dependencies then?

[davidanthoff]

Yes, that is what I did for the Query.jl branch. I could see this all being more elegant with some of the sub-project stuff that is coming? But for now this seems fine.

[davidanthoff]

One weird thing I ran across: when I ran the doctests as part of runtests.jl locally, I had to change all the doctests so that the output looked like 1×2 DataFrame instead of the 1×2 DataFrames.DataFrame. I then opened a PR against Query.jl to change that in all doctests (https://github.com/queryverse/Query.jl/pull/268/files), but in the CI run these now all failed.

Not sure what is going on, and I never understood when I have to add the module name in the output, but it looks a little as if one needs to do this differently when the doctests are run from runtests.jl?

[fredrikekre]

Thats because DataFrames was sometimes loaded in Main, see e.g. JuliaLang/julia#29466, you should be able to work around it by loading that into Main unconditionally.

[davidanthoff]

Thats because DataFrames was sometimes loaded in Main, see e.g. JuliaLang/julia#29466, you should be able to work around it by loading that into Main unconditionally.

Cool, thanks for pointing me to that, I always wanted to know what is going on there :)

So the story then is that in runtests.jl I have a using DataFrames, but not in docs/make.jl, and that leads to different output of the doctests depending on how you run them.

The short term fix is simple: I'll just add using DataFrames to docs/make.jl. As a general solution not great, but I guess this really needs to be fixed in base...

[tkf]

using Test, Documenter, MyPackage
@testset "Doctesting" begin
    @test doctest(MyPackage)
end

Can it be just doctest(MyPackage) instead of @testset "Doctesting"; @test doctest(MyPackage); end? It would be nice to get N failures for N failing doctests, rather than just a binary output.

(PS: It's great to see that doctests are decoupled from documentation!)

[mortenpi]

Can it be just doctest(MyPackage) instead of @testset "Doctesting"; @test doctest(MyPackage); end? It would be nice to get N failures for N failing doctests, rather than just a binary output.

That's a good idea, but I'll leave the implementation for another time, since it's a bit of work to make it happen (#1053).

[tkf]

How about defining it as doctest(m::Module) = @test doctest_impl(m) for now? This way, users don't have to change anything at the call sites if you make test results more detailed at some point.

[mortenpi]

This way, users don't have to change anything at the call sites if you make test results more detailed at some point.

That's a fair point. I am inclined to keep the doctest function as is though, so that there would a simple function you can call that just gives you true/false (and allows fixing the doctests).

What about @doctestset MyPackage? Would then currently expand into:

@testset "Doctests: MyPackage" begin
    @test doctest(MyPackage)
end

[tkf]

a simple function you can call that just gives you true/false

My honest impression is that it's not clear what the target user of doctest(::Module) :: Bool is. It's not handy for end-users because they need to use @testset and @test. It's not really an API suitable for testing utility packages because the output is too coarse-grained. But maybe this is not going to be a problem in practice. Error reporting from makedocs has been already quite useful.

FYI, I created a test helper tool https://github.com/tkf/Aqua.jl#quick-usage (though not related to doctest at all). I use the API like doctest(m::Module) = @test doctest_impl(m) I suggested above. I've never needed true/false output and I'm enjoying the simplicity of the test setup. Of course, this could be just due to my workflow.

[tkf]

What about @doctestset MyPackage?

I think it makes sense if you decide to go with doctest(::Module) :: Bool.

[mortenpi]

@tkf You won me over. The -> Bool API would not actually be particularly useful indeed. The only other use I can see for the doctest function, outside of runtests.jl, is in the REPL and there is no problem if it also returns a testset there (in fact, it might be preferred).

[tkf]

Great. Thank you for being open minded!