[v6,15/21] tests/qapi-schema: Add test of the rST QAPI doc-comment outputn

Commit Message

Peter Maydell Sept. 25, 2020, 4:23 p.m. UTC

Add a test of the rST output from the QAPI doc-comment generator,
similar to what we currently have that tests the Texinfo output.

This is a bit more awkward with Sphinx, because the generated
output is not 100% under our control the way the QAPI-to-Texinfo
generator was. However, in practice Sphinx's plaintext output
generation has been identical between at least Sphinx 1.6 and
3.0, so we use that. (The HTML output has had changes across
versions). We use an exact-match comparison check, with the
understanding that perhaps changes in a future Sphinx version
might require us to implement something more clever to cope
with variation in the output.

Signed-off-by: Peter Maydell <[e-mail]>
---
The meson.build changes are remarkably clunky, but this
appears to be unavoidable...
---
 tests/qapi-schema/doc-good.rst |   5 +
 tests/qapi-schema/doc-good.txt | 288 +++++++++++++++++++++++++++++++++
 tests/qapi-schema/meson.build  |  55 +++++++
 3 files changed, 348 insertions(+)
 create mode 100644 tests/qapi-schema/doc-good.rst
 create mode 100644 tests/qapi-schema/doc-good.txt

Comments

Markus Armbruster Sept. 29, 2020, 12:20 p.m. UTC | #1

In subject, s/outputn/output/

Peter Maydell <[e-mail]> writes:

> Add a test of the rST output from the QAPI doc-comment generator,
> similar to what we currently have that tests the Texinfo output.
>
> This is a bit more awkward with Sphinx, because the generated
> output is not 100% under our control the way the QAPI-to-Texinfo
> generator was. However, in practice Sphinx's plaintext output
> generation has been identical between at least Sphinx 1.6 and
> 3.0, so we use that. (The HTML output has had changes across
> versions). We use an exact-match comparison check, with the
> understanding that perhaps changes in a future Sphinx version
> might require us to implement something more clever to cope
> with variation in the output.
>
> Signed-off-by: Peter Maydell <[e-mail]>

It's not just the potential Sphinx version dependence that makes this
awkward.

We can no longer check what our doc generator does (at least not without
substantial additional coding), we can only check what it does together
with Sphinx.  We do so for one output format.

Our doc generator output could change in ways that are not visible in
the Sphinx output format we test, but are visible in some other output
format.

We choose to test plain text, because it has the lowest risk of unwanted
Sphinx version dependence, even though it probably has the highest risk
of "rendering stuff invisible".

Certainly better than nothing, and probably the best we can do now, but
let's capture the tradeoff in the commit message.  Perhaps:

  This is a bit more awkward with Sphinx, because the generated output
  is not 100% under our control the way the QAPI-to-Texinfo generator
  was. We can't observe the data we generate, only the Sphinx
  output. Two issues.

  One, the output can vary with the Sphinx version.  In practice
  Sphinx's plaintext output generation has been identical between at
  least Sphinx 1.6 and 3.0, so we use that. (The HTML output has had
  changes across versions). We use an exact-match comparison check, with
  the understanding that perhaps changes in a future Sphinx version
  might require us to implement something more clever to cope with
  variation in the output.

  Two, the test can only protect us from changes in the data we generate
  that are visible in plain text.

What do you think?

> ---
> The meson.build changes are remarkably clunky, but this
> appears to be unavoidable...
> ---
>  tests/qapi-schema/doc-good.rst |   5 +
>  tests/qapi-schema/doc-good.txt | 288 +++++++++++++++++++++++++++++++++
>  tests/qapi-schema/meson.build  |  55 +++++++
>  3 files changed, 348 insertions(+)
>  create mode 100644 tests/qapi-schema/doc-good.rst
>  create mode 100644 tests/qapi-schema/doc-good.txt
>
> diff --git a/tests/qapi-schema/doc-good.rst b/tests/qapi-schema/doc-good.rst
> new file mode 100644
> index 00000000000..1e4c23305a1
> --- /dev/null
> +++ b/tests/qapi-schema/doc-good.rst
> @@ -0,0 +1,5 @@
> +..
> +   Test Sphinx manual that pulls in the test schema file. We will generate
> +   a plain-text output file and compare it against a reference.
> +
> +.. qapi-doc:: tests/qapi-schema/doc-good.json
> diff --git a/tests/qapi-schema/doc-good.txt b/tests/qapi-schema/doc-good.txt
> new file mode 100644
> index 00000000000..6ca03d49d0d
> --- /dev/null
> +++ b/tests/qapi-schema/doc-good.txt
> @@ -0,0 +1,288 @@
> +Section
> +*******
> +
> +
> +Subsection
> +==========
> +
> +*with emphasis* "var" {in braces}
> +
> +* List item one
> +
> +* Two, multiple lines
> +
> +* Three Still in list
> +
> +Not in list
> +
> +* Second list Note: still in list
> +
> +Note: not in list
> +
> +1. Third list is numbered
> +
> +2. another item
> +
> +Returns: the King Since: the first age Notes:
> +
> +1. Lorem ipsum dolor sit amet
> +
> +2. Ut enim ad minim veniam
> +
> +Duis aute irure dolor
> +
> +Example:
> +
> +-> in <- out Examples: - *verbatim* - {braces}
> +
> +
> +"Enum" (Enum)
> +-------------
> +
> +
> +Values
> +~~~~~~
> +
> +"one" (**If: **"defined(IFONE)")
> +   The _one_ {and only}
> +
> +"two"
> +   Not documented
> +
> +
> +Features
> +~~~~~~~~
> +
> +"enum-feat"
> +   Also _one_ {and only}
> +
> +"two" is undocumented
> +
> +
> +If
> +~~
> +
> +"defined(IFCOND)"
> +
> +
> +"Base" (Object)
> +---------------
> +
> +
> +Members
> +~~~~~~~
> +
> +"base1": "Enum"
> +   the first member
> +
> +
> +"Variant1" (Object)
> +-------------------
> +
> +A paragraph
> +
> +Another paragraph (but no "var": line)
> +
> +
> +Members
> +~~~~~~~
> +
> +"var1": "string" (**If: **"defined(IFSTR)")
> +   Not documented
> +
> +
> +Features
> +~~~~~~~~
> +
> +"variant1-feat"
> +   a feature
> +
> +"member-feat"
> +   a member feature
> +
> +
> +"Variant2" (Object)
> +-------------------
> +
> +
> +"Object" (Object)
> +-----------------
> +
> +
> +Members
> +~~~~~~~
> +
> +The members of "Base"
> +The members of "Variant1" when "base1" is ""one""
> +The members of "Variant2" when "base1" is ""two"" (**If: **"IFTWO")
> +
> +Features
> +~~~~~~~~
> +
> +"union-feat1"
> +   a feature
> +
> +
> +"SugaredUnion" (Object)
> +-----------------------
> +
> +
> +Members
> +~~~~~~~
> +
> +"type"
> +   One of "one", "two"
> +
> +"data": "Variant1" when "type" is ""one""
> +"data": "Variant2" when "type" is ""two"" (**If: **"IFTWO")
> +
> +Features
> +~~~~~~~~
> +
> +"union-feat2"
> +   a feature
> +
> +
> +"Alternate" (Alternate)
> +-----------------------
> +
> +
> +Members
> +~~~~~~~
> +
> +"i": "int"
> +   an integer "b" is undocumented
> +
> +"b": "boolean"
> +   Not documented
> +
> +
> +Features
> +~~~~~~~~
> +
> +"alt-feat"
> +   a feature
> +
> +
> +Another subsection
> +==================
> +
> +
> +"cmd" (Command)
> +---------------
> +
> +
> +Arguments
> +~~~~~~~~~
> +
> +"arg1": "int"
> +   the first argument
> +
> +"arg2": "string" (optional)
> +   the second argument
> +
> +"arg3": "boolean"
> +   Not documented
> +
> +
> +Features
> +~~~~~~~~
> +
> +"cmd-feat1"
> +   a feature
> +
> +"cmd-feat2"
> +   another feature
> +
> +
> +Note
> +~~~~
> +
> +"arg3" is undocumented
> +
> +
> +Returns
> +~~~~~~~
> +
> +"Object"
> +
> +
> +TODO
> +~~~~
> +
> +frobnicate
> +
> +
> +Notes
> +~~~~~
> +
> +* Lorem ipsum dolor sit amet
> +
> +* Ut enim ad minim veniam
> +
> +Duis aute irure dolor
> +
> +
> +Example
> +~~~~~~~
> +
> +   -> in
> +   <- out
> +
> +
> +Examples
> +~~~~~~~~
> +
> +   - *verbatim*
> +   - {braces}
> +
> +
> +Since
> +~~~~~
> +
> +2.10
> +
> +
> +"cmd-boxed" (Command)
> +---------------------
> +
> +If you're bored enough to read this, go see a video of boxed cats
> +
> +
> +Arguments
> +~~~~~~~~~
> +
> +The members of "Object"
> +
> +Features
> +~~~~~~~~
> +
> +"cmd-feat1"
> +   a feature
> +
> +"cmd-feat2"
> +   another feature
> +
> +
> +Example
> +~~~~~~~
> +
> +   -> in
> +
> +   <- out
> +
> +
> +"EVT-BOXED" (Event)
> +-------------------
> +
> +
> +Arguments
> +~~~~~~~~~
> +
> +The members of "Object"
> +
> +Features
> +~~~~~~~~
> +
> +"feat3"
> +   a feature
> diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
> index 83a0a68389b..0c4a6a2936f 100644
> --- a/tests/qapi-schema/meson.build
> +++ b/tests/qapi-schema/meson.build
> @@ -224,3 +224,58 @@ qapi_doc = custom_target('QAPI doc',
>  test('QAPI doc', diff, args: ['-b', '-u', files('doc-good.texi'), qapi_doc[0].full_path()],
>       depends: qapi_doc,
>       suite: ['qapi-schema', 'qapi-doc'])
> +
> +# Test the document-comment document generation code by running a test schema
> +# file through Sphinx's plain-text builder and comparing the result against
> +# a golden reference. This is in theory susceptible to failures if Sphinx
> +# changes its output, but the text output has historically been very stable
> +# (no changes between Sphinx 1.6 and 3.0), so it is a better bet than
> +# texinfo or HTML generation, both of which have had changes. We might

Texinfo

> +# need to add more sophisticated logic here in future for some sort of
> +# fuzzy comparison if future Sphinx versions produce different text,
> +# but for now the simple comparison suffices.
> +qapi_doc_out = custom_target('QAPI rST doc',
> +                             output: ['doc-good.txt'],
> +                             input: files('doc-good.json', 'doc-good.rst'),

Gawk at my Meson ignorance...

Looks like this builds doc-good.txt from doc.good.json and doc-good.rst.

doc-good.txt is also a source file.  Works, because we use a separate
build tree.  Might be confusing, though.

> +                             build_by_default: build_docs,
> +                             depend_files: sphinx_extn_depends,
> +                             # We use -E to suppress Sphinx's caching, because
> +                             # we want it to always really run the QAPI doc
> +                             # generation code. It also means we don't
> +                             # clutter up the build dir with the cache.
> +                             command: [SPHINX_ARGS,
> +                                       '-b', 'text', '-E',
> +                                       '-c', meson.source_root() / 'docs',
> +                                       '-D', 'master_doc=doc-good',
> +                                       meson.current_source_dir(),
> +                                       meson.current_build_dir()])
> +
> +# Fix possible inconsistency in line endings in generated output and
> +# in the golden reference (which could otherwise cause test failures
> +# on Windows hosts). Unfortunately diff --strip-trailing-cr
> +# is GNU-diff only. The odd-looking perl is because we must avoid
> +# using an explicit '\' character in the command arguments to
> +# a custom_target(), as Meson will unhelpfully replace it with a '/'
> +# (https://github.com/mesonbuild/meson/issues/1564)

Rather disappointing.

> +qapi_doc_out_nocr = custom_target('QAPI rST doc newline-sanitized',
> +                                  output: ['doc-good.txt.nocr'],
> +                                  input: qapi_doc_out[0],
> +                                  build_by_default: build_docs,
> +                                  command: ['perl', '-pe', '$x = chr 13; s/$x$//', '@INPUT@'],
> +                                  capture: true)

I figure this strips \r from the build tree's doc-good.txt.

> +
> +qapi_doc_ref_nocr = custom_target('QAPI rST doc reference newline-sanitized',
> +                                  output: ['doc-good.ref.nocr'],
> +                                  input: files('doc-good.txt'),
> +                                  build_by_default: build_docs,
> +                                  command: ['perl', '-pe', '$x = chr 13; s/$x$//', '@INPUT@'],
> +                                  capture: true)

Uh, this strips it from the source tree's doc-good.txt, right?  Why is
that necessary?

Correcting myself: it *is* confusing.

> +
> +if build_docs
> +  # "full_path()" needed here to work around
> +  # https://github.com/mesonbuild/meson/issues/7585
> +  test('QAPI rST doc', diff, args: ['-u', qapi_doc_ref_nocr[0].full_path(),
> +                                    qapi_doc_out_nocr[0].full_path()],
> +       depends: [qapi_doc_ref_nocr, qapi_doc_out_nocr],
> +       suite: ['qapi-schema', 'qapi-doc'])
> +endif

Peter Maydell Sept. 29, 2020, 12:33 p.m. UTC | #2

On Tue, 29 Sep 2020 at 13:20, Markus Armbruster <[e-mail]> wrote:
>
> In subject, s/outputn/output/
>
> Peter Maydell <[e-mail]> writes:
>
> > Add a test of the rST output from the QAPI doc-comment generator,
> > similar to what we currently have that tests the Texinfo output.
> >
> > This is a bit more awkward with Sphinx, because the generated
> > output is not 100% under our control the way the QAPI-to-Texinfo
> > generator was. However, in practice Sphinx's plaintext output
> > generation has been identical between at least Sphinx 1.6 and
> > 3.0, so we use that. (The HTML output has had changes across
> > versions). We use an exact-match comparison check, with the
> > understanding that perhaps changes in a future Sphinx version
> > might require us to implement something more clever to cope
> > with variation in the output.
> >
> > Signed-off-by: Peter Maydell <[e-mail]>
>
> It's not just the potential Sphinx version dependence that makes this
> awkward.
>
> We can no longer check what our doc generator does (at least not without
> substantial additional coding), we can only check what it does together
> with Sphinx.  We do so for one output format.
>
> Our doc generator output could change in ways that are not visible in
> the Sphinx output format we test, but are visible in some other output
> format.
>
> We choose to test plain text, because it has the lowest risk of unwanted
> Sphinx version dependence, even though it probably has the highest risk
> of "rendering stuff invisible".
>
> Certainly better than nothing, and probably the best we can do now, but
> let's capture the tradeoff in the commit message.  Perhaps:
>
>   This is a bit more awkward with Sphinx, because the generated output
>   is not 100% under our control the way the QAPI-to-Texinfo generator
>   was. We can't observe the data we generate, only the Sphinx
>   output. Two issues.
>
>   One, the output can vary with the Sphinx version.  In practice
>   Sphinx's plaintext output generation has been identical between at
>   least Sphinx 1.6 and 3.0, so we use that. (The HTML output has had
>   changes across versions). We use an exact-match comparison check, with
>   the understanding that perhaps changes in a future Sphinx version
>   might require us to implement something more clever to cope with
>   variation in the output.
>
>   Two, the test can only protect us from changes in the data we generate
>   that are visible in plain text.
>
> What do you think?

Yes, seems worth recording that in the commit message (especially
now you've written the text :-)).

> > +# Test the document-comment document generation code by running a test schema
> > +# file through Sphinx's plain-text builder and comparing the result against
> > +# a golden reference. This is in theory susceptible to failures if Sphinx
> > +# changes its output, but the text output has historically been very stable
> > +# (no changes between Sphinx 1.6 and 3.0), so it is a better bet than
> > +# texinfo or HTML generation, both of which have had changes. We might
>
> Texinfo
>
> > +# need to add more sophisticated logic here in future for some sort of
> > +# fuzzy comparison if future Sphinx versions produce different text,
> > +# but for now the simple comparison suffices.
> > +qapi_doc_out = custom_target('QAPI rST doc',
> > +                             output: ['doc-good.txt'],
> > +                             input: files('doc-good.json', 'doc-good.rst'),
>
> Gawk at my Meson ignorance...
>
> Looks like this builds doc-good.txt from doc.good.json and doc-good.rst.
>
> doc-good.txt is also a source file.  Works, because we use a separate
> build tree.  Might be confusing, though.

Yes. We could change the name of the reference source file that
we have checked into the git repo if you wanted. (The output file
written by Sphinx has to be the same name as the input .rst file AFAICT.)

> > +                             build_by_default: build_docs,
> > +                             depend_files: sphinx_extn_depends,
> > +                             # We use -E to suppress Sphinx's caching, because
> > +                             # we want it to always really run the QAPI doc
> > +                             # generation code. It also means we don't
> > +                             # clutter up the build dir with the cache.
> > +                             command: [SPHINX_ARGS,
> > +                                       '-b', 'text', '-E',
> > +                                       '-c', meson.source_root() / 'docs',
> > +                                       '-D', 'master_doc=doc-good',
> > +                                       meson.current_source_dir(),
> > +                                       meson.current_build_dir()])
> > +
> > +# Fix possible inconsistency in line endings in generated output and
> > +# in the golden reference (which could otherwise cause test failures
> > +# on Windows hosts). Unfortunately diff --strip-trailing-cr
> > +# is GNU-diff only. The odd-looking perl is because we must avoid
> > +# using an explicit '\' character in the command arguments to
> > +# a custom_target(), as Meson will unhelpfully replace it with a '/'
> > +# (https://github.com/mesonbuild/meson/issues/1564)
>
> Rather disappointing.
>
> > +qapi_doc_out_nocr = custom_target('QAPI rST doc newline-sanitized',
> > +                                  output: ['doc-good.txt.nocr'],
> > +                                  input: qapi_doc_out[0],
> > +                                  build_by_default: build_docs,
> > +                                  command: ['perl', '-pe', '$x = chr 13; s/$x$//', '@INPUT@'],
> > +                                  capture: true)
>
> I figure this strips \r from the build tree's doc-good.txt.

Close; it turns either CRLF or LF into the host OS's
line-ending sequence (see below).

> > +qapi_doc_ref_nocr = custom_target('QAPI rST doc reference newline-sanitized',
> > +                                  output: ['doc-good.ref.nocr'],
> > +                                  input: files('doc-good.txt'),
> > +                                  build_by_default: build_docs,
> > +                                  command: ['perl', '-pe', '$x = chr 13; s/$x$//', '@INPUT@'],
> > +                                  capture: true)
>
> Uh, this strips it from the source tree's doc-good.txt, right?  Why is
> that necessary?

This is in case the user has a setup that eg has git
doing line-ending conversion on checkout somehow. As a
non-Windows user I opted to be belt-and-braces about
converting both files to a known-consistent line ending.
It's also necessary because the perl rune isn't really
"delete \r"; it's "delete any \r and then output the
line with the OS line ending" because the files it processes
are being read and written in text mode. So the output
will be \r\n on Windows and \n on Unix; the test passes
in both cases because both files have the same
line endings after conversion.

thanks
-- PMM

Markus Armbruster Sept. 29, 2020, 1:18 p.m. UTC | #3

Peter Maydell <[e-mail]> writes:

> On Tue, 29 Sep 2020 at 13:20, Markus Armbruster <[e-mail]> wrote:
>>
>> In subject, s/outputn/output/
>>
>> Peter Maydell <[e-mail]> writes:
>>
>> > Add a test of the rST output from the QAPI doc-comment generator,
>> > similar to what we currently have that tests the Texinfo output.
>> >
>> > This is a bit more awkward with Sphinx, because the generated
>> > output is not 100% under our control the way the QAPI-to-Texinfo
>> > generator was. However, in practice Sphinx's plaintext output
>> > generation has been identical between at least Sphinx 1.6 and
>> > 3.0, so we use that. (The HTML output has had changes across
>> > versions). We use an exact-match comparison check, with the
>> > understanding that perhaps changes in a future Sphinx version
>> > might require us to implement something more clever to cope
>> > with variation in the output.
>> >
>> > Signed-off-by: Peter Maydell <[e-mail]>
>>
>> It's not just the potential Sphinx version dependence that makes this
>> awkward.
>>
>> We can no longer check what our doc generator does (at least not without
>> substantial additional coding), we can only check what it does together
>> with Sphinx.  We do so for one output format.
>>
>> Our doc generator output could change in ways that are not visible in
>> the Sphinx output format we test, but are visible in some other output
>> format.
>>
>> We choose to test plain text, because it has the lowest risk of unwanted
>> Sphinx version dependence, even though it probably has the highest risk
>> of "rendering stuff invisible".
>>
>> Certainly better than nothing, and probably the best we can do now, but
>> let's capture the tradeoff in the commit message.  Perhaps:
>>
>>   This is a bit more awkward with Sphinx, because the generated output
>>   is not 100% under our control the way the QAPI-to-Texinfo generator
>>   was. We can't observe the data we generate, only the Sphinx
>>   output. Two issues.
>>
>>   One, the output can vary with the Sphinx version.  In practice
>>   Sphinx's plaintext output generation has been identical between at
>>   least Sphinx 1.6 and 3.0, so we use that. (The HTML output has had
>>   changes across versions). We use an exact-match comparison check, with
>>   the understanding that perhaps changes in a future Sphinx version
>>   might require us to implement something more clever to cope with
>>   variation in the output.
>>
>>   Two, the test can only protect us from changes in the data we generate
>>   that are visible in plain text.
>>
>> What do you think?
>
> Yes, seems worth recording that in the commit message (especially
> now you've written the text :-)).

:)

>> > +# Test the document-comment document generation code by running a test schema
>> > +# file through Sphinx's plain-text builder and comparing the result against
>> > +# a golden reference. This is in theory susceptible to failures if Sphinx
>> > +# changes its output, but the text output has historically been very stable
>> > +# (no changes between Sphinx 1.6 and 3.0), so it is a better bet than
>> > +# texinfo or HTML generation, both of which have had changes. We might
>>
>> Texinfo
>>
>> > +# need to add more sophisticated logic here in future for some sort of
>> > +# fuzzy comparison if future Sphinx versions produce different text,
>> > +# but for now the simple comparison suffices.
>> > +qapi_doc_out = custom_target('QAPI rST doc',
>> > +                             output: ['doc-good.txt'],
>> > +                             input: files('doc-good.json', 'doc-good.rst'),
>>
>> Gawk at my Meson ignorance...
>>
>> Looks like this builds doc-good.txt from doc.good.json and doc-good.rst.
>>
>> doc-good.txt is also a source file.  Works, because we use a separate
>> build tree.  Might be confusing, though.
>
> Yes. We could change the name of the reference source file that
> we have checked into the git repo if you wanted. (The output file
> written by Sphinx has to be the same name as the input .rst file AFAICT.)

I'll see what I can do (and thanks for the hint).

>> > +                             build_by_default: build_docs,
>> > +                             depend_files: sphinx_extn_depends,
>> > +                             # We use -E to suppress Sphinx's caching, because
>> > +                             # we want it to always really run the QAPI doc
>> > +                             # generation code. It also means we don't
>> > +                             # clutter up the build dir with the cache.
>> > +                             command: [SPHINX_ARGS,
>> > +                                       '-b', 'text', '-E',
>> > +                                       '-c', meson.source_root() / 'docs',
>> > +                                       '-D', 'master_doc=doc-good',
>> > +                                       meson.current_source_dir(),
>> > +                                       meson.current_build_dir()])
>> > +
>> > +# Fix possible inconsistency in line endings in generated output and
>> > +# in the golden reference (which could otherwise cause test failures
>> > +# on Windows hosts). Unfortunately diff --strip-trailing-cr
>> > +# is GNU-diff only. The odd-looking perl is because we must avoid
>> > +# using an explicit '\' character in the command arguments to
>> > +# a custom_target(), as Meson will unhelpfully replace it with a '/'
>> > +# (https://github.com/mesonbuild/meson/issues/1564)
>>
>> Rather disappointing.
>>
>> > +qapi_doc_out_nocr = custom_target('QAPI rST doc newline-sanitized',
>> > +                                  output: ['doc-good.txt.nocr'],
>> > +                                  input: qapi_doc_out[0],
>> > +                                  build_by_default: build_docs,
>> > +                                  command: ['perl', '-pe', '$x = chr 13; s/$x$//', '@INPUT@'],
>> > +                                  capture: true)
>>
>> I figure this strips \r from the build tree's doc-good.txt.
>
> Close; it turns either CRLF or LF into the host OS's
> line-ending sequence (see below).
>
>> > +qapi_doc_ref_nocr = custom_target('QAPI rST doc reference newline-sanitized',
>> > +                                  output: ['doc-good.ref.nocr'],
>> > +                                  input: files('doc-good.txt'),
>> > +                                  build_by_default: build_docs,
>> > +                                  command: ['perl', '-pe', '$x = chr 13; s/$x$//', '@INPUT@'],
>> > +                                  capture: true)
>>
>> Uh, this strips it from the source tree's doc-good.txt, right?  Why is
>> that necessary?
>
> This is in case the user has a setup that eg has git
> doing line-ending conversion on checkout somehow. As a
> non-Windows user I opted to be belt-and-braces about
> converting both files to a known-consistent line ending.
> It's also necessary because the perl rune isn't really
> "delete \r"; it's "delete any \r and then output the
> line with the OS line ending" because the files it processes
> are being read and written in text mode. So the output
> will be \r\n on Windows and \n on Unix; the test passes
> in both cases because both files have the same
> line endings after conversion.

Uff.  Thanks!

Reviewed-by: Markus Armbruster <[e-mail]>

Patch

diff --git a/tests/qapi-schema/doc-good.rst b/tests/qapi-schema/doc-good.rst
new file mode 100644
index 00000000000..1e4c23305a1
--- /dev/null
+++ b/tests/qapi-schema/doc-good.rst
@@ -0,0 +1,5 @@ 
+..
+   Test Sphinx manual that pulls in the test schema file. We will generate
+   a plain-text output file and compare it against a reference.
+
+.. qapi-doc:: tests/qapi-schema/doc-good.json
diff --git a/tests/qapi-schema/doc-good.txt b/tests/qapi-schema/doc-good.txt
new file mode 100644
index 00000000000..6ca03d49d0d
--- /dev/null
+++ b/tests/qapi-schema/doc-good.txt
@@ -0,0 +1,288 @@ 
+Section
+*******
+
+
+Subsection
+==========
+
+*with emphasis* "var" {in braces}
+
+* List item one
+
+* Two, multiple lines
+
+* Three Still in list
+
+Not in list
+
+* Second list Note: still in list
+
+Note: not in list
+
+1. Third list is numbered
+
+2. another item
+
+Returns: the King Since: the first age Notes:
+
+1. Lorem ipsum dolor sit amet
+
+2. Ut enim ad minim veniam
+
+Duis aute irure dolor
+
+Example:
+
+-> in <- out Examples: - *verbatim* - {braces}
+
+
+"Enum" (Enum)
+-------------
+
+
+Values
+~~~~~~
+
+"one" (**If: **"defined(IFONE)")
+   The _one_ {and only}
+
+"two"
+   Not documented
+
+
+Features
+~~~~~~~~
+
+"enum-feat"
+   Also _one_ {and only}
+
+"two" is undocumented
+
+
+If
+~~
+
+"defined(IFCOND)"
+
+
+"Base" (Object)
+---------------
+
+
+Members
+~~~~~~~
+
+"base1": "Enum"
+   the first member
+
+
+"Variant1" (Object)
+-------------------
+
+A paragraph
+
+Another paragraph (but no "var": line)
+
+
+Members
+~~~~~~~
+
+"var1": "string" (**If: **"defined(IFSTR)")
+   Not documented
+
+
+Features
+~~~~~~~~
+
+"variant1-feat"
+   a feature
+
+"member-feat"
+   a member feature
+
+
+"Variant2" (Object)
+-------------------
+
+
+"Object" (Object)
+-----------------
+
+
+Members
+~~~~~~~
+
+The members of "Base"
+The members of "Variant1" when "base1" is ""one""
+The members of "Variant2" when "base1" is ""two"" (**If: **"IFTWO")
+
+Features
+~~~~~~~~
+
+"union-feat1"
+   a feature
+
+
+"SugaredUnion" (Object)
+-----------------------
+
+
+Members
+~~~~~~~
+
+"type"
+   One of "one", "two"
+
+"data": "Variant1" when "type" is ""one""
+"data": "Variant2" when "type" is ""two"" (**If: **"IFTWO")
+
+Features
+~~~~~~~~
+
+"union-feat2"
+   a feature
+
+
+"Alternate" (Alternate)
+-----------------------
+
+
+Members
+~~~~~~~
+
+"i": "int"
+   an integer "b" is undocumented
+
+"b": "boolean"
+   Not documented
+
+
+Features
+~~~~~~~~
+
+"alt-feat"
+   a feature
+
+
+Another subsection
+==================
+
+
+"cmd" (Command)
+---------------
+
+
+Arguments
+~~~~~~~~~
+
+"arg1": "int"
+   the first argument
+
+"arg2": "string" (optional)
+   the second argument
+
+"arg3": "boolean"
+   Not documented
+
+
+Features
+~~~~~~~~
+
+"cmd-feat1"
+   a feature
+
+"cmd-feat2"
+   another feature
+
+
+Note
+~~~~
+
+"arg3" is undocumented
+
+
+Returns
+~~~~~~~
+
+"Object"
+
+
+TODO
+~~~~
+
+frobnicate
+
+
+Notes
+~~~~~
+
+* Lorem ipsum dolor sit amet
+
+* Ut enim ad minim veniam
+
+Duis aute irure dolor
+
+
+Example
+~~~~~~~
+
+   -> in
+   <- out
+
+
+Examples
+~~~~~~~~
+
+   - *verbatim*
+   - {braces}
+
+
+Since
+~~~~~
+
+2.10
+
+
+"cmd-boxed" (Command)
+---------------------
+
+If you're bored enough to read this, go see a video of boxed cats
+
+
+Arguments
+~~~~~~~~~
+
+The members of "Object"
+
+Features
+~~~~~~~~
+
+"cmd-feat1"
+   a feature
+
+"cmd-feat2"
+   another feature
+
+
+Example
+~~~~~~~
+
+   -> in
+
+   <- out
+
+
+"EVT-BOXED" (Event)
+-------------------
+
+
+Arguments
+~~~~~~~~~
+
+The members of "Object"
+
+Features
+~~~~~~~~
+
+"feat3"
+   a feature
diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
index 83a0a68389b..0c4a6a2936f 100644
--- a/tests/qapi-schema/meson.build
+++ b/tests/qapi-schema/meson.build
@@ -224,3 +224,58 @@  qapi_doc = custom_target('QAPI doc',
 test('QAPI doc', diff, args: ['-b', '-u', files('doc-good.texi'), qapi_doc[0].full_path()],
      depends: qapi_doc,
      suite: ['qapi-schema', 'qapi-doc'])
+
+# Test the document-comment document generation code by running a test schema
+# file through Sphinx's plain-text builder and comparing the result against
+# a golden reference. This is in theory susceptible to failures if Sphinx
+# changes its output, but the text output has historically been very stable
+# (no changes between Sphinx 1.6 and 3.0), so it is a better bet than
+# texinfo or HTML generation, both of which have had changes. We might
+# need to add more sophisticated logic here in future for some sort of
+# fuzzy comparison if future Sphinx versions produce different text,
+# but for now the simple comparison suffices.
+qapi_doc_out = custom_target('QAPI rST doc',
+                             output: ['doc-good.txt'],
+                             input: files('doc-good.json', 'doc-good.rst'),
+                             build_by_default: build_docs,
+                             depend_files: sphinx_extn_depends,
+                             # We use -E to suppress Sphinx's caching, because
+                             # we want it to always really run the QAPI doc
+                             # generation code. It also means we don't
+                             # clutter up the build dir with the cache.
+                             command: [SPHINX_ARGS,
+                                       '-b', 'text', '-E',
+                                       '-c', meson.source_root() / 'docs',
+                                       '-D', 'master_doc=doc-good',
+                                       meson.current_source_dir(),
+                                       meson.current_build_dir()])
+
+# Fix possible inconsistency in line endings in generated output and
+# in the golden reference (which could otherwise cause test failures
+# on Windows hosts). Unfortunately diff --strip-trailing-cr
+# is GNU-diff only. The odd-looking perl is because we must avoid
+# using an explicit '\' character in the command arguments to
+# a custom_target(), as Meson will unhelpfully replace it with a '/'
+# (https://github.com/mesonbuild/meson/issues/1564)
+qapi_doc_out_nocr = custom_target('QAPI rST doc newline-sanitized',
+                                  output: ['doc-good.txt.nocr'],
+                                  input: qapi_doc_out[0],
+                                  build_by_default: build_docs,
+                                  command: ['perl', '-pe', '$x = chr 13; s/$x$//', '@INPUT@'],
+                                  capture: true)
+
+qapi_doc_ref_nocr = custom_target('QAPI rST doc reference newline-sanitized',
+                                  output: ['doc-good.ref.nocr'],
+                                  input: files('doc-good.txt'),
+                                  build_by_default: build_docs,
+                                  command: ['perl', '-pe', '$x = chr 13; s/$x$//', '@INPUT@'],
+                                  capture: true)
+
+if build_docs
+  # "full_path()" needed here to work around
+  # https://github.com/mesonbuild/meson/issues/7585
+  test('QAPI rST doc', diff, args: ['-u', qapi_doc_ref_nocr[0].full_path(),
+                                    qapi_doc_out_nocr[0].full_path()],
+       depends: [qapi_doc_ref_nocr, qapi_doc_out_nocr],
+       suite: ['qapi-schema', 'qapi-doc'])
+endif