diff mbox series

tests/qapi-schema: Tidy up pylint warnings and advice

Message ID 20231025092925.1785934-1-armbru@redhat.com
State New
Headers show
Series tests/qapi-schema: Tidy up pylint warnings and advice | expand

Commit Message

Markus Armbruster Oct. 25, 2023, 9:29 a.m. UTC
Pylint warns:

    tests/qapi-schema/test-qapi.py:139:13: W1514: Using open without explicitly specifying an encoding (unspecified-encoding)
    tests/qapi-schema/test-qapi.py:143:13: W1514: Using open without explicitly specifying an encoding (unspecified-encoding)

Add encoding='utf-8'.

Pylint advises:

    tests/qapi-schema/test-qapi.py:143:13: R1732: Consider using 'with' for resource-allocating operations (consider-using-with)

Silence this by returning the value directly.

Pylint advises:

    tests/qapi-schema/test-qapi.py:221:4: R1722: Consider using sys.exit() (consider-using-sys-exit)
    tests/qapi-schema/test-qapi.py:226:4: R1722: Consider using sys.exit() (consider-using-sys-exit)

Sure, why not.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/qapi-schema/test-qapi.py | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Comments

John Snow Nov. 3, 2023, 3:21 a.m. UTC | #1
On Wed, Oct 25, 2023 at 5:36 AM Markus Armbruster <armbru@redhat.com> wrote:
>
> Pylint warns:
>
>     tests/qapi-schema/test-qapi.py:139:13: W1514: Using open without explicitly specifying an encoding (unspecified-encoding)
>     tests/qapi-schema/test-qapi.py:143:13: W1514: Using open without explicitly specifying an encoding (unspecified-encoding)
>
> Add encoding='utf-8'.

This one I think actually matters in some cases. Some esoteric cases
where the platform default isn't UTF-8. Might happen on Windows where
it wants to use UTF-16 or UCS-2 or whatever.

>
> Pylint advises:
>
>     tests/qapi-schema/test-qapi.py:143:13: R1732: Consider using 'with' for resource-allocating operations (consider-using-with)
>
> Silence this by returning the value directly.

Smart!

>
> Pylint advises:
>
>     tests/qapi-schema/test-qapi.py:221:4: R1722: Consider using sys.exit() (consider-using-sys-exit)
>     tests/qapi-schema/test-qapi.py:226:4: R1722: Consider using sys.exit() (consider-using-sys-exit)
>
> Sure, why not.

I have no idea why this matters, but I habitually use the sys.exit() form ...

https://docs.python.org/3.11/library/constants.html#constants-added-by-the-site-module

Oh, that's weird. Well, Sure, why not.

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

Reviewed-by: John Snow <jsnow@redhat.com>

> ---
>  tests/qapi-schema/test-qapi.py | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
> index d58c31f539..14f7b62a44 100755
> --- a/tests/qapi-schema/test-qapi.py
> +++ b/tests/qapi-schema/test-qapi.py
> @@ -136,12 +136,11 @@ def test_frontend(fname):
>  def open_test_result(dir_name, file_name, update):
>      mode = 'r+' if update else 'r'
>      try:
> -        fp = open(os.path.join(dir_name, file_name), mode)
> +        return open(os.path.join(dir_name, file_name), mode, encoding='utf-8')
>      except FileNotFoundError:
>          if not update:
>              raise
> -        fp = open(os.path.join(dir_name, file_name), 'w+')
> -    return fp
> +    return open(os.path.join(dir_name, file_name), 'w+', encoding='utf-8')
>
>
>  def test_and_diff(test_name, dir_name, update):
> @@ -218,9 +217,9 @@ def main(argv):
>          test_name = os.path.splitext(base_name)[0]
>          status |= test_and_diff(test_name, dir_name, args.update)
>
> -    exit(status)
> +    sys.exit(status)
>
>
>  if __name__ == '__main__':
>      main(sys.argv)
> -    exit(0)
> +    sys.exit(0)
> --
> 2.41.0
>
>
Markus Armbruster Nov. 3, 2023, 5:33 a.m. UTC | #2
Markus Armbruster <armbru@redhat.com> writes:

> Pylint warns:
>
>     tests/qapi-schema/test-qapi.py:139:13: W1514: Using open without explicitly specifying an encoding (unspecified-encoding)
>     tests/qapi-schema/test-qapi.py:143:13: W1514: Using open without explicitly specifying an encoding (unspecified-encoding)
>
> Add encoding='utf-8'.
>
> Pylint advises:
>
>     tests/qapi-schema/test-qapi.py:143:13: R1732: Consider using 'with' for resource-allocating operations (consider-using-with)
>
> Silence this by returning the value directly.
>
> Pylint advises:
>
>     tests/qapi-schema/test-qapi.py:221:4: R1722: Consider using sys.exit() (consider-using-sys-exit)
>     tests/qapi-schema/test-qapi.py:226:4: R1722: Consider using sys.exit() (consider-using-sys-exit)
>
> Sure, why not.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Queued.
diff mbox series

Patch

diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index d58c31f539..14f7b62a44 100755
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -136,12 +136,11 @@  def test_frontend(fname):
 def open_test_result(dir_name, file_name, update):
     mode = 'r+' if update else 'r'
     try:
-        fp = open(os.path.join(dir_name, file_name), mode)
+        return open(os.path.join(dir_name, file_name), mode, encoding='utf-8')
     except FileNotFoundError:
         if not update:
             raise
-        fp = open(os.path.join(dir_name, file_name), 'w+')
-    return fp
+    return open(os.path.join(dir_name, file_name), 'w+', encoding='utf-8')
 
 
 def test_and_diff(test_name, dir_name, update):
@@ -218,9 +217,9 @@  def main(argv):
         test_name = os.path.splitext(base_name)[0]
         status |= test_and_diff(test_name, dir_name, args.update)
 
-    exit(status)
+    sys.exit(status)
 
 
 if __name__ == '__main__':
     main(sys.argv)
-    exit(0)
+    sys.exit(0)