[LTP] [PATCH v4] doc: add tests catalog page

[Petr Vorel]

Hi Andrea,

generally LGTM, thanks for working on it.

1) clickable (referenced) keys

I asked keys (e.g. needs_root) to be referenced to be clickable to the
relevant API.  This allows reader to click and read relevant info about the key.

2) git commit description of referenced tags

Perl code was able to take git description.  E.g. instead of plain "d2f007dbe7e4"
it got:
d2f007dbe7e4 ("userns: also map extents in the reverse map to kernel IDs")

I understand you don't want to bother with it, but it was more descriptive.
I guess using github API or simple fetching web page on git.kernel.org and
take content of "commit-subject" would work, but it'd be slow and we could hit
some access limits from the provider. I can have look into this afterwards (I
don't want to block this issue).

3) CSS fix of some tables

Code on tables with keys and values (e.g. needs_kconfigs) looks ugly, when there
are more items.  E.g. needs_kconfigs at icmp_rate_limit01, which has:
CONFIG_VETH
CONFIG_USER_NS=y
CONFIG_NET_NS=y

The ugly part is margin-bottom: 24px; from class="line-block".
Single item line uses <p></p>.

I looked into docs (https://tables-with-sphinx.readthedocs.io/en/latest/),
but than I realized it's a simple CSS issue. It can be fixed by adding
into doc/_static/custom.css:

/* remove margin for multiline cells */
.rst-content table td div.line-block {
    margin-bottom: 0;
}

I can send it as a separate patch if you don't want to bother with it.

> +def _generate_tags_table(tags):
> +    """
> +    Generate the tags table from tags hash.
> +    """

nit: conf.py is getting slightly big. I need to remember that public functions
are these which generates stats.html and test_catalog.html. Maybe having
functions or setup() which references them on the top would help reader to start
with relevant code.
...

> +def _generate_setup_table(keys):
> +    """
> +    Generate the table with test setup configuration.
> +    """
> +    exclude = [
This list is slightly confusing for a reader. I would expect there will be no
options in generated docs nor tags. Obviously they are processed separately.

> +        'child_needs_reinit',
> +        'needs_checkpoints',
> +        'resource_files',
Maybe add this? e.g. creat07 requires also creat07_child. That might be useful
for somebody to see that test needs another LTP binary.

> +        'save_restore',
This is useful, it shows which /proc/sys files are touched.

> +        'forks_child',
> +        'hugepages',
I know that these are cryptic:
.hugepages = {1, TST_NEEDS},
.hugepages = {TST_NO_HUGEPAGES},
but trying to filter out info may cause people prefer using 'git grep' sources
instead of searching in 'ctrl+F' in this nice documentation.

> +        'options',
> +        'timeout',
> +        'runtime',
Cyril already asked for runtime, IMHO important to have.

> +        'ulimit',
Also ulimit is useful.


> +        'fname',
> +        'tags',
> +        'doc',
> +    ]
...
> +def generate_test_catalog(_):
> +    """
> +    Generate the test catalog from ltp.json metadata file.
> +    """
> +    output = '_static/tests.rst'
> +    metadata_file = '../metadata/ltp.json'
> +    text = [
> +        '.. warning::',
> +        '    The following catalog has been generated using LTP metadata',
> +        '    which is including only tests using the new :ref:`LTP C API`.',
> +        '    For this reason, some old tests might be missing from the list.',
Besides the fact that *all* tests using legacy API are skipped I don't think
user is interested why they aren't included. I would just note that only tests
which use new :ref:`LTP C API` are here listed.

> +        ''
> +    ]
> +
> +    metadata = None
> +    with open(metadata_file, 'r', encoding='utf-8') as data:
> +        metadata = json.load(data)
> +
> +    timeout_def = metadata['defaults']['timeout']
> +
> +    for test_name, conf in metadata['tests'].items():
> +        text.extend([
> +            f'{test_name}',
> +            len(test_name) * '-'
> +        ])
> +
> +        # source url location
> +        test_fname = conf.get('fname', None)
> +        if test_fname:
> +            text.extend([
> +                '',
> +                f"`source <{ltp_repo_base_url}/{test_fname}>`__",
> +                ''
> +            ])
> +
> +        # test description
> +        desc = conf.get('doc', None)
> +        if desc:
> +            desc_text = []
> +            for line in desc:
> +                if line.startswith("[Description]"):
> +                    desc_text.append("**Description**")
I suggested about deleting all [Description] from the code. I see no value in
it, but it forces reader to scroll more (it prolongs the page).

> +                elif line.startswith("[Algorithm]"):
> +                    desc_text.append("**Algorithm**")
> +                else:
> +                    desc_text.append(line)
NOTE: because there are more titles in "[..]" (there can be anything, why not
just replace \[(.*)\] with **\1** ?
> +
> +            text.extend([
> +                '\n'.join(desc_text),
> +            ])
> +
> +
> +        # developer information
For me would be more obvious comment something like: "parse struct tst_test content".

Kind regards,
Petr

> +        text.append('')
> +        text.extend(_generate_setup_table(conf))
> +        text.append('')
> +
> +        # small separator between tests
> +        text.extend([
> +            '',
> +            '.. raw:: html',
> +            '',
> +            '    <hr>',
> +            '',
> +        ])
> +
> +    with open(output, 'w+', encoding='utf-8') as new_tests:
> +        new_tests.write('\n'.join(text))
...