Re: [PATCH v3 8/8] qapi: switch to use QEMU_TEST_REGENERATE env var

["Daniel P. Berrangé" <berrange@redhat.com>]

On Mon, Aug 25, 2025 at 02:01:35PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > On Fri, Aug 08, 2025 at 08:46:10AM +0200, Markus Armbruster wrote:
> >> 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.
> >> >
> >> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> [...]
> 
> >> > 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.
> 
> Which existing print()s do you mean?

I was referring to the ones in this method

    except OSError as err:
        print("%s: can't open '%s': %s"
              % (sys.argv[0], err.filename, err.strerror),
              file=sys.stderr)
        return 2

..

    except OSError as err:
        print("%s: can't write '%s': %s"
              % (sys.argv[0], err.filename, err.strerror),
              file=sys.stderr)


though I notice now they are using 'sys.argv[0]' which is not the
same as 'test_name', as well as using stderr. So the consistency
isn't actually helped

> 
> >
> >> 
> >> >            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 :|