[igt-dev] [RFC] IGT Subtest Documentation

[Kalamarz, Lukasz]

On Tue, 2019-05-28 at 15:29 +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:
>   ...
> ```

LGTM:) Had one question. With test description that will include
reasoning for a given testcase/binary, should it be duplicated in
commit message?
----
Lukasz

> 
> 
> ## 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.
> _______________________________________________
> igt-dev mailing list
> igt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev