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

[Meta] Test Rationale Use Cases & Implementation #387

Open
hwine opened this issue Oct 9, 2020 · 8 comments
Open

[Meta] Test Rationale Use Cases & Implementation #387

hwine opened this issue Oct 9, 2020 · 8 comments
Milestone

Comments

@hwine
Copy link
Contributor

hwine commented Oct 9, 2020

In slack discussions and #383 & #384, we think of good usages for text to explain the "why" of a test (also called "rationale" in some places). However, there may be multiple implementations (mark.rationale and doc string), and there may be impacts on the desire to leverage existing Sphinx tooling to auto-document the code.

This issue is to capture use cases and discussion around these features before deciding on an implementation strategy. The current implementation strategies include:

Both the "rationale" and the "description" are output to the frost JSON for downstream use, but both are considered optional.

Action Needed

Each use case is a separate comment below -- folks can vote on whether or not they are "in scope" using reactions. Of course, folks should also add missing use cases, as well! We can then reach a decision ensuring the approach will support the remaining use cases.

Implementation concerns

The perceived tensions are:

  • Sphinx has no built in mechanism to access pytest marks while building the documentation. It does have mechanisms to access the docstring.
  • Sphinx has automation to document functions in modules, using the docstring information.
  • It is unclear which source would be easier for the frost wrapper to extract to display on the terminal.
@hwine
Copy link
Contributor Author

hwine commented Oct 9, 2020

UC-1

As a frost user, I want to understand which tests are appropriate for my run from the cli in order to minimize the chance of confusing the interpretation of the output.

@hwine
Copy link
Contributor Author

hwine commented Oct 9, 2020

UC-2

As an ops person doing triage on frost output, I want to minimize the chance of misinterpreting the results, to ensure the highest priority findings are mitigated first.

@hwine
Copy link
Contributor Author

hwine commented Oct 9, 2020

UC-3

As a frost configurator, I want to easily select which tests I wish to include in the profile I'm creating, so that I don't generate output of low value to my situation.

@hwine
Copy link
Contributor Author

hwine commented Oct 9, 2020

UC-4

As a frost developer, I want access to function definitions in the rendered documentation, so I don't misuse the API.

@smarnach
Copy link
Contributor

I've always used the docstrings of test functions to document the rationale of the test. What else would I put in the docstring? A test function does not have any interface to document, since it's never explicitly called anywhere, so we can only document what it does and why. Using the docstring for this purpose doesn't feel like "overloading" the docstring to me, but rather like using it exactly for what it is intended.

Docstrings are included in the Sphinx documentation by default, and they are also easy to access programmatically for other purposes. Using pytest.mark.rationale() instead makes it more difficult to include the rationales in the Sphinx documentation, so it using docstrings looks easier to me.

AJ said on Slack that the original reason for using a pytest mark was that it was way easier to pass that string along into the results from a pytest run. Does this refere to this code?

"rationale": meta["rationale"],

If so, we could look into how difficult it would be to pass on the docstring instead.

@hwine
Copy link
Contributor Author

hwine commented Oct 12, 2020

Agreed on needing to look into the code more -- aiui, that location is "far away" from the actual test function, so introspection may be a challenge (but doable, I'm sure).

Let's simplify and defer this decision until we understand better what we want in the docs from #389. I think we have all the variant use cases on the table now, so we can make trade-offs knowledgeably.

@smarnach
Copy link
Contributor

We could also go for some hybrid approach, e.g. by making the test loader copy the docstring into the rationale mark, or by providing a custom decorator for this purpose.

@ajvb
Copy link
Contributor

ajvb commented Oct 14, 2020

I agree with figuring out the use-case(s) first, but just for future reference both the mark's and the docstrings are currently passed into the test results.

marks:

markers = {n.name: serialize_marker(n) for n in get_node_markers(item)}

docstrings:

description = item._obj.__doc__ and clean_docstring(item._obj.__doc__)

@ajvb ajvb added this to the 0.6.0 milestone Nov 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants