[igt-dev] [PATCH i-g-t 5/5] docs: Embed subtest descriptions in the documentation

[Arkadiusz Hiler]

On Mon, Jun 24, 2019 at 11:00:17AM +0300, Ser, Simon wrote:
> On Mon, 2019-06-17 at 13:54 +0300, Arkadiusz Hiler wrote:
> > This rewrites generate_description_xml in Python, so that we generate
> > properly escaped XML. The switch also makes the code more manageable.
> > 
> > Changes in the generated docbook:
> > 
> > 1. subtests are not simply listed anymore, they are now another (sub)section
> > 
> > 2. subtests are now linkable,
> >    e.g. docs/igt-kms-tests.html#kms_hdmi_inject at inject-4k
> > 
> > 3. subtest's section now includes output of --describe
> > 
> > Python is required already by gtk-doc and we are not using anything
> > other than the standard library.
> > 
> > Cc: Daniel Vetter <daniel at ffwll.ch>
> > Cc: Petri Latvala <petri.latvala at intel.com>
> > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler at intel.com>
> > ---
> >  .../igt-gpu-tools/generate_description_xml.py | 112 ++++++++++++++++++
> >  .../igt-gpu-tools/generate_description_xml.sh |  46 -------
> >  docs/reference/igt-gpu-tools/meson.build      |   2 +-
> >  3 files changed, 113 insertions(+), 47 deletions(-)
> >  create mode 100755 docs/reference/igt-gpu-tools/generate_description_xml.py
> >  delete mode 100644 docs/reference/igt-gpu-tools/generate_description_xml.sh
> > 
> > diff --git a/docs/reference/igt-gpu-tools/generate_description_xml.py b/docs/reference/igt-gpu-tools/generate_description_xml.py
> > new file mode 100755
> > index 00000000..87af945d
> > --- /dev/null
> > +++ b/docs/reference/igt-gpu-tools/generate_description_xml.py
> > @@ -0,0 +1,112 @@
> > +#!/usr/bin/env python3
> > +
> > +import re
> > +import sys
> > +import os.path
> > +import subprocess
> > +import xml.etree.cElementTree as ET
> > +
> > +from collections import namedtuple
> > +
> > +Subtest = namedtuple("Subtest", "name description")
> 
> I'm not sure I understand "name description". Shouldn't it just be
> "Description"?

Python's way of defining named tuples is a bit weird. The first argument
is the name of the "class" and the second one is space separated list of
fields.

So this will create 'Subtest' named tuple for holding two elements.
First element will be accessible via .name and second via .description.

> > +KEYWORDS=re.compile(r'\b(invalid|hang|swap|thrash|crc|tiled|tiling|rte|ctx|render|blt|bsd|vebox|exec|rpm)\b')
> > +
> > +
> > +def get_testlist(path):
> > +    "read binaries' names from test-list.txt"
> > +    with open(path, 'r') as f:
> > +        assert(f.readline() == "TESTLIST\n")
> > +        tests = f.readline().strip().split(" ")
> > +        assert(f.readline() == "END TESTLIST\n")
> > +
> > +    return tests
> > +
> > +
> > +def keywordize(root, text, keywords):
> > +    "set text for root element and wrap KEYWORDS in a <acronym>"
> > +    matches = list(keywords.finditer(text))
> > +
> > +    if not matches:
> > +        root.text = text
> > +        return
> > +
> > +    pos = 0
> > +    last_element = None
> > +    root.text = ""
> > +
> > +    for match in matches:
> > +        if match.start() > pos:
> > +            to_append = text[pos:match.start()]
> > +
> > +            if last_element == None:
> > +                root.text += to_append
> > +            else:
> > +                last_element.tail += to_append
> > +
> > +        last_element = ET.SubElement(root, "acronym")
> > +        last_element.tail = ""
> > +        last_element.text=match.group()
> > +        pos = match.end()
> 
> This misses the text after the last match.

good catch

last_element.tail = text[pos:]

should solve that

> > +def get_subtests(testdir, test):
> > +    "execute test and get subtests with their descriptions via --describe"
> > +    output = []
> > +    full_test_path = os.path.join(testdir, test)
> > +    proc = subprocess.run([full_test_path, "--describe"], stdout=subprocess.PIPE)
> 
> Add check=True to make sure the exit status is 0.

This made me realize that for --list-subtests and --describe we are
returning 79 to denote that there are no subtests here (i.e. for
igt_simple_main).

% build/tests/core_getclient --describe
Tests the DRM_IOCTL_GET_CLIENT ioctl.

% echo $?
79

And we still utilize the description from there:

<div class="refsect2">
<a name="core_getclient"></a><h3>core_getclient</h3>
<p>Tests the DRM_IOCTL_GET_CLIENT ioctl.</p>
</div>

> > +    description = ""
> > +    current_subtest = None
> > +
> > +    for line in proc.stdout.decode().splitlines():
> > +        if line.startswith("SUB "):
> > +            output += [Subtest(current_subtest, description)]
> 
> Should only do that if description != "".
> 
> > +            description = ""
> > +            current_subtest = line.split(' ')[1]
> > +        else:
> > +            description += line
> > +
> > +    output += [Subtest(current_subtest, description)]
> 
> Ditto, although this one is more like a nit.

I am not sure why we should care?

We are producing obnoxious "NO DOCUMENTATION!" if the subtest is not
described, and if we still somehow manage to produce nothing then at
least we would list that test in the docs.

-- 
Cheers,
Arek