Re: [PATCH v4 7/7] qapi: switch to use QEMU_TEST_REGENERATE env var

[Markus Armbruster <armbru@redhat.com>]

Daniel P. Berrangé <berrange@redhat.com> writes:

> The QAPI_TEST_UPDATE env var can be set when running the QAPI
> schema tests to regenerate the reference output. For consistent
> naming with the tracetool test, change the env var name to
> QEMU_TEST_REGENERATE.
>
> The test is modified to provide a hint about use of the new
> env var and it is also added to the developer documentation.document its usage.
>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  docs/devel/testing/main.rst    | 12 ++++++++++++
>  tests/qapi-schema/test-qapi.py |  7 +++++--
>  2 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/docs/devel/testing/main.rst b/docs/devel/testing/main.rst
> index 11f05c0006..0662766b5c 100644
> --- a/docs/devel/testing/main.rst
> +++ b/docs/devel/testing/main.rst
> @@ -178,6 +178,18 @@ parser (either fixing a bug or extending/modifying the syntax). To do this:
>  
>    ``qapi-schema += foo.json``
>  
> +The reference output can be automatically updated to match the latest QAPI
> +code generator by running the tests with the QEMU_TEST_REGENERATE environment
> +variable set.
> +
> +.. code::
> +
> +   QEMU_TEST_REGENERATE=1 make check-qapi-schema
> +
> +The resulting changes must be reviewed by the author to ensure they match
> +the intended results before adding the updated reference output to the
> +same commit that alters the generator code.
> +
>  .. _tracetool-tests:
>  
>  Tracetool tests

Appreciated, thanks!

> diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
> index 4be930228c..cf7fb8a6df 100755
> --- a/tests/qapi-schema/test-qapi.py
> +++ b/tests/qapi-schema/test-qapi.py
> @@ -165,7 +165,7 @@ def test_and_diff(test_name, dir_name, update):
>      if actual_out == expected_out and actual_err == expected_err:
>          return 0
>  
> -    print("%s %s" % (test_name, 'UPDATE' if update else 'FAIL'),
> +    print("%s: %s" % (test_name, 'UPDATE' if update else 'FAIL'),

I would've left this out just to keep the patch focused.  Definitely not
worth a respin now.

>            file=sys.stderr)
>      out_diff = difflib.unified_diff(expected_out, actual_out, outfp.name)
>      err_diff = difflib.unified_diff(expected_err, actual_err, errfp.name)
> @@ -173,6 +173,9 @@ def test_and_diff(test_name, dir_name, update):
>      sys.stdout.writelines(err_diff)
>  
>      if not update:
> +        print(("\n%s: set QEMU_TEST_REGENERATE=1 to recreate reference output" +
> +               "if the QAPI schema generator was intentionally changed") % test_name,
> +              file=sys.stderr)
>          return 1
>  
>      try:
> @@ -197,7 +200,7 @@ def main(argv):
>      parser.add_argument('-d', '--dir', action='store', default='',
>                          help="directory containing tests")
>      parser.add_argument('-u', '--update', action='store_true',
> -                        default='QAPI_TEST_UPDATE' in os.environ,
> +                        default='QEMU_TEST_REGENERATE' in os.environ,
>                          help="update expected test results")
>      parser.add_argument('tests', nargs='*', metavar='TEST', action='store')
>      args = parser.parse_args()

Reviewed-by: Markus Armbruster <armbru@redhat.com>