| Message ID | 20210922125619.670673-2-armbru@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tests/qapi-schema: Minor tooling improvements | expand |
On Wed, Sep 22, 2021 at 8:56 AM Markus Armbruster <armbru@redhat.com> wrote: > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > tests/qapi-schema/test-qapi.py | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tests/qapi-schema/test-qapi.py > b/tests/qapi-schema/test-qapi.py > index 73cffae2b6..2e384f5efd 100755 > --- a/tests/qapi-schema/test-qapi.py > +++ b/tests/qapi-schema/test-qapi.py > @@ -154,7 +154,7 @@ def test_and_diff(test_name, dir_name, update): > errfp = open(os.path.join(dir_name, test_name + '.err'), mode) > expected_out = outfp.readlines() > expected_err = errfp.readlines() > - except IOError as err: > + except OSError as err: > print("%s: can't open '%s': %s" > % (sys.argv[0], err.filename, err.strerror), > file=sys.stderr) > @@ -180,7 +180,7 @@ def test_and_diff(test_name, dir_name, update): > errfp.truncate(0) > errfp.seek(0) > errfp.writelines(actual_err) > - except IOError as err: > + except OSError as err: > print("%s: can't write '%s': %s" > % (sys.argv[0], err.filename, err.strerror), > file=sys.stderr) > -- > 2.31.1 > > If you're happy with the expanded scope of the exception-catcher, I am too. Reviewed-by: John Snow <jsnow@redhat.com>
John Snow <jsnow@redhat.com> writes: > On Wed, Sep 22, 2021 at 8:56 AM Markus Armbruster <armbru@redhat.com> wrote: > >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> tests/qapi-schema/test-qapi.py | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/tests/qapi-schema/test-qapi.py >> b/tests/qapi-schema/test-qapi.py >> index 73cffae2b6..2e384f5efd 100755 >> --- a/tests/qapi-schema/test-qapi.py >> +++ b/tests/qapi-schema/test-qapi.py >> @@ -154,7 +154,7 @@ def test_and_diff(test_name, dir_name, update): >> errfp = open(os.path.join(dir_name, test_name + '.err'), mode) >> expected_out = outfp.readlines() >> expected_err = errfp.readlines() >> - except IOError as err: >> + except OSError as err: >> print("%s: can't open '%s': %s" >> % (sys.argv[0], err.filename, err.strerror), >> file=sys.stderr) >> @@ -180,7 +180,7 @@ def test_and_diff(test_name, dir_name, update): >> errfp.truncate(0) >> errfp.seek(0) >> errfp.writelines(actual_err) >> - except IOError as err: >> + except OSError as err: >> print("%s: can't write '%s': %s" >> % (sys.argv[0], err.filename, err.strerror), >> file=sys.stderr) >> -- >> 2.31.1 >> >> > If you're happy with the expanded scope of the exception-catcher, I am too. https://docs.python.org/3.6/library/exceptions.html has Changed in version 3.3: EnvironmentError, IOError, WindowsError, socket.error, select.error and mmap.error have been merged into OSError, and the constructor may return a subclass. and The following exceptions are kept for compatibility with previous versions; starting from Python 3.3, they are aliases of OSError. exception EnvironmentError exception IOError exception WindowsError Only available on Windows. So unless I'm misunderstanding something (which is quite possible), we're catching exactly the same exceptions as before, we just switch to their preferred name. > Reviewed-by: John Snow <jsnow@redhat.com> Thanks!
On 9/23/21 11:33, Markus Armbruster wrote: > John Snow <jsnow@redhat.com> writes: > >> On Wed, Sep 22, 2021 at 8:56 AM Markus Armbruster <armbru@redhat.com> wrote: >> >>> Signed-off-by: Markus Armbruster <armbru@redhat.com> >>> --- >>> tests/qapi-schema/test-qapi.py | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/tests/qapi-schema/test-qapi.py >>> b/tests/qapi-schema/test-qapi.py >>> index 73cffae2b6..2e384f5efd 100755 >>> --- a/tests/qapi-schema/test-qapi.py >>> +++ b/tests/qapi-schema/test-qapi.py >>> @@ -154,7 +154,7 @@ def test_and_diff(test_name, dir_name, update): >>> errfp = open(os.path.join(dir_name, test_name + '.err'), mode) >>> expected_out = outfp.readlines() >>> expected_err = errfp.readlines() >>> - except IOError as err: >>> + except OSError as err: >>> print("%s: can't open '%s': %s" >>> % (sys.argv[0], err.filename, err.strerror), >>> file=sys.stderr) >>> @@ -180,7 +180,7 @@ def test_and_diff(test_name, dir_name, update): >>> errfp.truncate(0) >>> errfp.seek(0) >>> errfp.writelines(actual_err) >>> - except IOError as err: >>> + except OSError as err: >>> print("%s: can't write '%s': %s" >>> % (sys.argv[0], err.filename, err.strerror), >>> file=sys.stderr) >>> -- >>> 2.31.1 >>> >>> >> If you're happy with the expanded scope of the exception-catcher, I am too. > > https://docs.python.org/3.6/library/exceptions.html has > > Changed in version 3.3: EnvironmentError, IOError, WindowsError, > socket.error, select.error and mmap.error have been merged into > OSError, and the constructor may return a subclass. > > and > > The following exceptions are kept for compatibility with previous > versions; starting from Python 3.3, they are aliases of OSError. > > exception EnvironmentError > > exception IOError > > exception WindowsError > > Only available on Windows. With that information amended to the description: Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > So unless I'm misunderstanding something (which is quite possible), > we're catching exactly the same exceptions as before, we just switch to > their preferred name. > >> Reviewed-by: John Snow <jsnow@redhat.com> > > Thanks! > >
On Thu, Sep 23, 2021 at 5:33 AM Markus Armbruster <armbru@redhat.com> wrote: > John Snow <jsnow@redhat.com> writes: > > > On Wed, Sep 22, 2021 at 8:56 AM Markus Armbruster <armbru@redhat.com> > wrote: > > > >> Signed-off-by: Markus Armbruster <armbru@redhat.com> > >> --- > >> tests/qapi-schema/test-qapi.py | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/tests/qapi-schema/test-qapi.py > >> b/tests/qapi-schema/test-qapi.py > >> index 73cffae2b6..2e384f5efd 100755 > >> --- a/tests/qapi-schema/test-qapi.py > >> +++ b/tests/qapi-schema/test-qapi.py > >> @@ -154,7 +154,7 @@ def test_and_diff(test_name, dir_name, update): > >> errfp = open(os.path.join(dir_name, test_name + '.err'), mode) > >> expected_out = outfp.readlines() > >> expected_err = errfp.readlines() > >> - except IOError as err: > >> + except OSError as err: > >> print("%s: can't open '%s': %s" > >> % (sys.argv[0], err.filename, err.strerror), > >> file=sys.stderr) > >> @@ -180,7 +180,7 @@ def test_and_diff(test_name, dir_name, update): > >> errfp.truncate(0) > >> errfp.seek(0) > >> errfp.writelines(actual_err) > >> - except IOError as err: > >> + except OSError as err: > >> print("%s: can't write '%s': %s" > >> % (sys.argv[0], err.filename, err.strerror), > >> file=sys.stderr) > >> -- > >> 2.31.1 > >> > >> > > If you're happy with the expanded scope of the exception-catcher, I am > too. > > https://docs.python.org/3.6/library/exceptions.html has > > Changed in version 3.3: EnvironmentError, IOError, WindowsError, > socket.error, select.error and mmap.error have been merged into > OSError, and the constructor may return a subclass. > > and > > The following exceptions are kept for compatibility with previous > versions; starting from Python 3.3, they are aliases of OSError. > > exception EnvironmentError > > exception IOError > > exception WindowsError > > Only available on Windows. > > So unless I'm misunderstanding something (which is quite possible), > we're catching exactly the same exceptions as before, we just switch to > their preferred name. > > > Reviewed-by: John Snow <jsnow@redhat.com> > > Thanks! > > Yeah, I suppose the 3.3 upgrade already "expanded" the coverage here, so you aren't expanding anything. It's just an expansion of intent in the source code, if that distinction makes sense. The code is obviously fine so far as I can tell. My RB stands! --js
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py index 73cffae2b6..2e384f5efd 100755 --- a/tests/qapi-schema/test-qapi.py +++ b/tests/qapi-schema/test-qapi.py @@ -154,7 +154,7 @@ def test_and_diff(test_name, dir_name, update): errfp = open(os.path.join(dir_name, test_name + '.err'), mode) expected_out = outfp.readlines() expected_err = errfp.readlines() - except IOError as err: + except OSError as err: print("%s: can't open '%s': %s" % (sys.argv[0], err.filename, err.strerror), file=sys.stderr) @@ -180,7 +180,7 @@ def test_and_diff(test_name, dir_name, update): errfp.truncate(0) errfp.seek(0) errfp.writelines(actual_err) - except IOError as err: + except OSError as err: print("%s: can't write '%s': %s" % (sys.argv[0], err.filename, err.strerror), file=sys.stderr)
Signed-off-by: Markus Armbruster <armbru@redhat.com> --- tests/qapi-schema/test-qapi.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)