On Fri, Aug 08, 2025 at 08:46:10AM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berra...@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.
> >
> > Signed-off-by: Daniel P. Berrangé <berra...@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..fe233fcf7a 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
>
> Not sure about the comma. I'm not a native speaker, though.
Yeah, the comma is redundant
> > +same commit that alters the generator code.
> > +
> > .. _tracetool-tests:
> >
> > Tracetool tests
> > 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'),
>
> Is there a need for this, or is it just drive-by polishing?
Just making it more consistent in style with other error print()
statements we have in the file, as well as this new one I'm
adding.
>
> > 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()
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
