[igt-dev] [RFC] IGT Subtest Documentation

[Daniel Vetter]

On Tue, May 28, 2019 at 03:29:34PM +0300, Arkadiusz Hiler wrote:
> ## The Idea
> 
> Most of the tests are quite easy to follow but yet it's hard to tell why they
> are doing what they are doing if you are not the author. Their names are just
> not enough.
> 
> That results in archaeological diggings using `git blame` and mailing list
> archives just to grasp the purpose. Commit history gets diluted over time as
> improvements and refactors amass, making it progressively harder to find the
> original intention.
> 
> **The idea** is to provide concise commit-message-quality explanation along
> the code, and make the code more readable and available as a part of
> documentation.
> 
> "Describe the spirit of the test, not the letter."
> 
> 
> ### Why not more detailed descriptions?
> 
> Translating C to English is a bad idea, as it effectively duplicates the
> code. Both *implementations* will diverge and increase the maintenance
> burden.
> 
> If parts of code are hard to follow or do not ready easy enough they should
> be refactored and/or annotated with normal C comments.
> 
> Any extra clarifications, diagrams, etc. should be added as free-form
> comments to the code, which will be accessible through documentation.
> 
> If someone is unable to read *annotated* IGT source code, English transpill
> will not help.
> 
> 
> ### What are the benefits of this approach?
> 
> 1. The descriptions available on command line via
>    `--describe [PATTERN]`
> 
> 2. Generated docs will contain the test name with description for an easy
>    online lookup. The doc will also link to source file on the exact line of
>    the subtest for quick reference.
> 
> 
> ## The C API
> 
> ```c
> igt_describe_test(char *dsc);
> 
> igt_describe(char *dsc);
> igt_describe_f(char *fmt, ...);
> ```
> 
> 
> ### Example Code
> 
> ```c
> /* tests/frob_knob_basic.c */
> #include "igt.h"
> #include "frob.h"
> 
> igt_describe_test("Check basic functionality of the frob IOCTL on a knob");
> igt_main
> {
> 	igt_describe("Basic test making sure that the frobbing IOCTL "
> 		     "succeeds on the given knob with no arguments.\n"):
> 	igt_subtest_group {
> 		igt_subtest("frob-the-knob-a")
> 			test_frob(KNOB_A);
> 
> 		igt_subtest("frob-the-knob-b")
> 			test_frob(KNOB_B);
> 	}
> 
> 	igt_describe("Frobbing and Skewing are used by asynchronous outer "
> 		     "space, so one should never stall on the other."):
> 	igt_subtest_group {
> 		igt_describe("Make sure that frob does not stall for a skew by "
> 			     "squeezing many frobs in-between two skews.\n"):
> 		igt_subtest("frob-vs-skew")
> 			test_frob_vs_skew();
> 
> 		igt_describe("Make sure that skew does not stall for a frob "
> 			     "by doing many skews and frobs simultaneously "
> 			     "and making sure that we maintain sensible "
> 			     "throughput of each.\n"):
> 		igt_subtest("skew-vs-frob");
> 			test_skew_vs_frob();
> 	}
> }
> ```
> 
> 
> ### Example Output
> 
> ```
> $ ./frob_knob_basic --describe
> 
> Check basic functionality of the frob IOCT on a knob.
> 
> SUB frob-the-knob-a:
>   Basic test making sure that the frobbing IOCTL succeed on the given knob
>   with no arguments.
> 
> SUB frob-the-knob-b:
>   Basic test making sure that the frobbing IOCTL succeed on the given knob
>   with no arguments.
> 
> SUB frob-vs-skew:
>   Frobbing and Skewing are used by asynchronous outer space, so one should
>   never stall on the other.
> 
>   Make sure that frob does not stall for a skew by squeezing many frobs
>   in-between two skews.
> 
> SUB skew-vs-frob:
>   Frobbing and Skewing are used by asynchronous outer space, so one should
>   never stall on the other.
> 
>   Make sure that skew does not stall for a frob by doing many skews and frobs
>   simultaneously and making sure that we maintain sensible throughput of
>   each.
> ```
> 
> 
> ### Generated Documentation
> 
> We are already generating dcoumnetation by invoking `--list-subtests` and
> `--help-description`. We can easily extend that to use the new `--describe`.
> 
> With a bit of gymnastics we can pull the whole `.c` file into the
> documentation and link subtests to the line they are introduced on (either in
> the local copy of the source or in the gitlab).
> 
> This needs small extensions to the --describe format as follows:
> 
> ```
> SUB frob-the-knob-a test/frob-knob-basic.c:9:
>   ...
> ```
> 
> 
> ## FAQ
> 
> **Q**: `igt_describe_test()` before main? How?
> **A**: it's a rename of `IGT_TEST_DESCRIPTION()` macro we already use.
> 
> **Q**: I've seen that somewhere before...
> **A**: https://patchwork.freedesktop.org/patch/171220/
> 
> **Q**: What has happened with the other implementation mentioned by Daniel?
> **A**: I was unable to find more details on it. Seems like it has not emerged.

Feels like a natural extension of IGT_TEST_DESCRIPTION, and given how
little uptake that had I don't think we should aim for more. Also, better
chance that people won't just reword what the C code does in bad English,
but actually try to explain the aim of a testcase.

+1 me

Yes that's a complete flip from me, but the gtkdoc thing died and no one
cares maybe this gets things started. I still think the
IGT_TEST_DESCRIPTION stuff is rather hard on the eyes :-)

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch