Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jldoctest support #15

Open
darsnack opened this issue Mar 12, 2021 · 4 comments
Open

jldoctest support #15

darsnack opened this issue Mar 12, 2021 · 4 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@darsnack
Copy link
Contributor

Does Publish plan to support jldoctest blocks in docstrings? This is useful for making sure any examples can always run.

@MichaelHatherly MichaelHatherly added enhancement New feature or request help wanted Extra attention is needed labels Mar 12, 2021
@MichaelHatherly
Copy link
Owner

MichaelHatherly commented Mar 12, 2021

Yes, some form of doctest-like feature does need to be added, thanks for opening this. It's worth taking some time to decide exactly how it should behave once actually implemented. I've never been 100% happy with how jldoctest ended up when it got implemented for Documenter and I think it could probably be done better with hindsight, in particular the manual intervention required to make tests pass when some repl printing changes either in Base or a package can be a bit of a pain to deal with and the heuristics used for determining whether a test passes can be hard to get 100% right every time. So here's probably a decent place to start gathering together the shortfalls of jldoctest so that whatever comes out of here is decidedly better for everyone, feel free to list your grievances (or things you like).

I've been playing around with the idea of turning the whole doctest feature on it's head a bit, i.e: write out the examples as @test expressions in the docstring's codeblock and then have the package just expand them into nicely formatted REPL examples if they actually pass, thus skipping the writer needing to copy-paste stuff from the REPL themselves (if that's still the workflow people are using?). Basically it would look like this:

module MyPackage

"""
# Examples

```jldoctest
a = 1
b = 2; # semi-colon would suppress output as usual
c = a + b
@test c == 3 # test lines wouldn't appear in the final output
```
"""
function foo end

# 
doctest!(@__MODULE__) # or maybe just @doctest! would search docstrings in the module and
                      # expand and test them when precompiling the package.

end

doctest! would probably live in DocStringExtensions.jl. The final docstring that gets printed to users would then be:

# Examples

```julia-repl
julia> a = 1
1

julia> b = 2; # semi-colon would suppress output as usual

julia> c = a + b
3
```

Kind of a rough idea that's just in my head at the moment, so I've not really thought about the downsides of it yet.

@darsnack
Copy link
Contributor Author

Nice, thanks for the response.

That design looks great for examples in docstrings where we do some kind of check to indicate to the reader that "yes, this function returns the thing you think it returns." I would characterize that as about 40% of the jldoctest examples that I write. The other 60% are like the a = 1 style stuff. These examples are quick-start snippets to show the user how to use the highest-level of the API. The reason for wrapping them in a jldoctest is as a sanity check to make sure that nothing about the user-facing API breaks if I change something in the backend of the package.

I'm not entirely sure if there is value beyond testing custom shows for being able to include the REPL output. If there was an error thrown by the user-facing API, then the regular tests would catch that too. Somehow, the REPL output is both a blessing and a curse. You have to keep it up to date with printing changes, but sometimes those print outs can catch more subtle bugs that your regular tests miss. It would be a form of redundancy that we might lose with the above approach.

@MichaelHatherly
Copy link
Owner

Yeah, jldoctest has pretty much become an ad-hoc show test framework. I think it would be good to make it a little more explicit exactly what within a doctest actually needs to have it's show representation tested in that way, perhaps just with something analogous to:

@test sprint(show, a) == "1"

but with as much of that repetition abstracted away so that it doesn't actually get in the way of writing these things:

"""
```jldoctest
a = 1
@test_show a "1"
```
"""

the nice thing about making it explicit is you aren't forced to test every value's show in each doctest whether it belongs to your package or some other, or Base. Adjusting tonnes of doctests when Base happens to change it's display of a vector slightly gets annoying quickly.

@ericphanson
Copy link

I like how doctests currently work; I think copy-pasting can be avoided by doctest(MyPackage; fix=true). That’s not always 100% robust but I think it’s a good API, because you can see the change and then commit it if it’s right, so to me the improvement would be making it more robust. And you could have automated PRs updating doctests etc.

Some annoyance can be avoided by running doctests only on one version of Julia (eg as part of building docs, with strict=:doctest in Documenter or some other way with Publish).

To me, something like @test_show doesn’t feel like a good solution because often I want to test the printing of big structs, and then indentation can be an issue, as well as just having a whole bunch of output in the source that isn’t going to be displayed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
3 participants