crate-python icon indicating copy to clipboard operation
crate-python copied to clipboard

Improve doctests vs. rendered documentation

Open amotl opened this issue 3 years ago • 4 comments

Hi there,

this is a small proposal I would like to make. Please upvote if you think it should be improved, or downvote if you think it should be left in the current state. Comments are also welcome.

With kind regards, Andreas.

Introduction

At https://github.com/crate/crate-python/pull/442#discussion_r957012464, we had a conversation about the src/crate/client/doctests/cursor.txt file. On the other hand, there is also docs/query.rst. Both have in common that they demonstrate how to query data from CrateDB.

Problem

Apparently, cursor.txt is being tested ^1, but query.rst is not. Vice versa, query.rst is part of the documentation ^2, but cursor.txt is not.

Proposal

We may think about having both: Add query.rst to the test suite, and add cursor.txt to the rendered documentation. This will both improve code quality (query.rst will be validated) and the documentation (parts of the tests in cursor.txt may be interesting to readers).

amotl avatar Sep 28 '22 22:09 amotl

@mfussenegger stated at https://github.com/crate/crate-python/pull/442#discussion_r991056478:

We shouldn't write doctests to ensure the client functions. Doctests are to ensure examples in the documention work. The primary concern is the documentation, not the test coverage it gives. If we use this for testing, we should convert it to regular python tests.

I absolutely agree with this. I only had the feeling that the use of doctests for testing was actually intended. I will be happy to steward the test suite into this direction.

amotl avatar Oct 11 '22 12:10 amotl

Apparently, cursor.txt is being tested, but query.rst is not. Vice versa, query.rst is part of the documentation, but cursor.txt is not.

Coming from this situation, and aiming to implement your suggestion, I think the best way forward would be to:

  • Convert all the test cases from cursor.txt into regular Python tests.
  • Add query.rst to the doctests, to make sure the documentation is valid.

amotl avatar Oct 11 '22 12:10 amotl

Hi. My proposal to steward this into a better shape, is #464, still WIP. Early comments are welcome.

amotl avatar Oct 13 '22 13:10 amotl

Status

I think #464 is ready now. On this page, the new documentation section is rendered as a preview on RTD:

Backlog

  • [ ] Improve the overall flow of the documenation.
  • [ ] Rework the documentation about BLOBs. Currently, it is scattered between docs/blobs.rst, docs/by-example/http.rst, and docs/by-example/blob.rst, and apparently outlines different access variants or stages. Let's sort this out.
  • [ ] Add more reStructuredText documents from docs/*.rst to the test suite as well. Currently, they are not, mostly because they do not contain executable examples.

amotl avatar Dec 08 '22 20:12 amotl

On behalf of this patch, the situation has been improved significantly already.

  • https://github.com/crate/crate-python/pull/464

One backlog item to carry over:

  • GH-627

Closing this.

amotl avatar Jun 17 '24 20:06 amotl