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

Automating keeping documentation examples up to date #711

Closed
DRMacIver opened this issue Jul 2, 2017 · 11 comments
Closed

Automating keeping documentation examples up to date #711

DRMacIver opened this issue Jul 2, 2017 · 11 comments
Assignees
Labels
tests/build/CI about testing or deployment *of* Hypothesis

Comments

@DRMacIver
Copy link
Member

I really like that we check documentation examples are up to date in CI. I think it's a great addition to the build and very much want to keep it.

But as it stands it's basically untenable, because it means that every time I make a change to to something low level in Hypothesis that has an impact on the data generation I have to run the following loop:

  1. Run "make documentation"
  2. Find the one example that it tells me about by hand
  3. Manually copy and paste the test output into that one example

In particular if I do something that changes the distribution of integers() I have to run this loop several dozen times. This is both slow (the docs build process is ~10 seconds) and incredibly annoying. I'm doing this for #710 at the moment and I basically never want to do it again ever. This makes me want to not touch Hypothesis internals at all, which is pretty counterproductive.

So in order to get the good without the annoyance, we need some form of automation.

Options I can think of:

  1. Some sort of hacky script that parses doctest output and puts it in place.
  2. Use sphinx-autorun instead of doctest. This has the downside that we won't see the output in diffs, and it will make it much harder to detect documentation breakages, but is probably the most viable option.
  3. Delete most of the examples of doing this from the documentation so the number of iterations of the loop is less annoying.

I also propose adding an item to the guide to the tune of "to the greatest extent possible, any checks for which the fixes could be automated should be automated".

@DRMacIver DRMacIver added the tests/build/CI about testing or deployment *of* Hypothesis label Jul 2, 2017
@The-Compiler
Copy link
Contributor

The-Compiler commented Jul 2, 2017

FWIW pytest uses regendoc for this. I'm not sure how suitable this is for projects other than pytest (and it's pretty bare-bones at the moment, check pytest's doc makefile for how to use it). It seems to work pretty well for pytest though.

cc @RonnyPfannschmidt

@DRMacIver
Copy link
Member Author

As an additional data-point about the shape of what a solution to this should look like, I've just changed something in #710 on the basis of how bad it made the documentation look. This is a strong argument in favour of making the diffs visible in the changelog.

It also means I have to redo all of the bits I've manually fixed so far. :-(

@DRMacIver
Copy link
Member Author

@The-Compiler Interesting. Does this actually do the doctest stuff already? It looks more like it's for keeping version numbers and stuff up to date.

@The-Compiler
Copy link
Contributor

Hmm, right, I don't think it does doctest-like things, it's more intended for $ invocations of a tool, which probably isn't your usecase.

See e.g. https://github.com/pytest-dev/pytest/pull/1780/files for a bigger change - it just reruns all invocations and updates the output.

@DRMacIver
Copy link
Member Author

Yeah, the specific use-case is that we have a lot of console examples that are currently run via doctests, but their output necessarily changes when Hypothesis's implementation does.

@DRMacIver
Copy link
Member Author

Initial investigation of rundoc is that it's a bit on the primitive side. In particular there isn't any sort of global setup like we currently do for doctest. OTOH it is also very small so we could just maintain a patched fork....

@DRMacIver
Copy link
Member Author

Yeah, the current situation is completely untenable I'm afraid. I've just discovered another problem: Because it's relying on the global random number generator, any change in testing has arbitrary knock on effects for other changes in testing.

I would much rather have out of date docs than deal with this for any changes I'm making to Hypothesis, so I'm going to open a pull request disabling doctests until we can resolve this issue. Sorry.

@DRMacIver
Copy link
Member Author

Helpful suggestion from Twitter: https://twitter.com/meejah/status/881545340360376320

Basically if we split out the doctest examples into their own files and include them in sphinx with a literalinclude, we can then run doctest on those files rather than on the documentation.

It's then much easier to update those examples. We'd still have to write a small script to do it, but it should be comparatively straightforward I think.

DRMacIver added a commit that referenced this issue Jul 2, 2017
@Zac-HD Zac-HD self-assigned this Jul 3, 2017
@Zac-HD
Copy link
Member

Zac-HD commented Jul 3, 2017

I have a strong preference for option # 1, to catch examples that we deprecate or outright break. Nobody wants to read the docs and see an example followed by a massive traceback - even if it's honest, fixing the example is better.

I also prefer to keep the examples inline as they are now for ease of editing, so I volunteer as tribute to write that update script I mentioned in #585 when converting them in the first place. Medium priority, but I assume everyone else is happy to just leave them disabled until I finish (or give up)?

@DRMacIver
Copy link
Member Author

I volunteer as tribute to write that update script I mentioned in #585 when converting them in the first place.

We accept your sacrifice. 😈

More seriously, if you're willing to do it then I agree that this would be the best option. Thanks.

Medium priority, but I assume everyone else is happy to just leave them disabled until I finish (or give up)?

Yup, fine by me.

@Zac-HD
Copy link
Member

Zac-HD commented Nov 3, 2017

Turns out that the implementation of the sphinx doctest builder is far wartier than I'd expected, due to shoehorning doctests into a builder API, but we still want to use it for nice setup and discovery features.

So it looks like the best way forward is to... parse the output of sphinx-build -b doctest docs /dev/null and do a simple find/replace. (Yep.) Challenges:

  • the doctest location reporting is garbage - while a file and line number is given, for doctests included from a docstring it's the .rst file it's included in... and the line number relative to the start of the docstring. I'll have to write up a good report for upstream at some point, but for now some combination of known filenames and grepping should do 😕
    If it's not in the reported location, we search hypothesis.strategies then give up. But I've left a note pointing to the inspect module which could do this correctly in ~20x as much code, for anyone who wants to do it properly later!
  • dealing correctly with indentation
    Does ensuring we always match the terminal output count? (ie four spaces for everything)
  • be 'just robust enough' - my script is going to be fairly fragile against later changes and won't cover edge cases like broken setup, but if it works for most of our use-cases I won't care.
    Haha. "It works for me" (for now), and that's good enough until it breaks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests/build/CI about testing or deployment *of* Hypothesis
Projects
None yet
Development

No branches or pull requests

3 participants