Re: [PATCH v9 0/6] Rework iotests/check
On 16/02/2021 02.49, John Snow wrote: On 1/26/21 10:15 AM, Vladimir Sementsov-Ogievskiy wrote: OK, thanks for handling it! When will we move to python 3.7? [...] As for RHEL/CentOS, I think it's in the same shape right now. It's 3.6-based, but I don't know if there's an optional 3.7+ package or not. There is a python 3.8 package available for RHEL8. And according to the QEMU support policy, we will soon (IIRC in May 2021) stop supporting RHEL7. Thomas
Re: [PATCH v9 0/6] Rework iotests/check
On 1/26/21 10:15 AM, Vladimir Sementsov-Ogievskiy wrote: OK, thanks for handling it! When will we move to python 3.7? "I don't know, but it seems like a very long time." The nominal EOL for Python 3.6 is this December; see https://www.python.org/dev/peps/pep-0494/ Debian 10 ships 3.7. Fedora has appropriate packages going back to F29 and possibly earlier. OpenSuSE tumbleweed offers 3.8 and 3.9. Ubuntu 18.04 LTS offers 3.7 and 3.8 packages. Ubuntu 20.04 LTS offers 3.7 and 3.8 packages. OpenSUSE 15.2 ... ships with Python 3.6, and I don't see an option to install anything more modern. This distribution is supported (both by us and by SuSE) until 2021-12-31. As of 2020-12-29, OpenSuSE 15.3 appears to be set to ship Python 3.6: "python3-3.6.12-3.70.1.x86_64.rpm". It is set to ship 2021-07-07, and does not have an estimated EOL date yet. SLES 15 ... I'm not sure and can't seem to find that information quickly. https://www.suse.com/releasenotes/x86_64/SUSE-SLES/15-SP1/ which was published at the end of 2020, suggests that Python 3 support(?) is new(?) to some extent. https://packagehub.suse.com/packages/python3/ suggests that SLES 15.2 is Python 3.6-based. I can't tell if they offer the optional addition of Python 3.7 on either SLES or OpenSuSE. Would love to know. As for RHEL/CentOS, I think it's in the same shape right now. It's 3.6-based, but I don't know if there's an optional 3.7+ package or not. --js Note: our configure script doesn't try several canonical sources of Python interpreters, we just try to use "python3". In the case of multiple python installations, we'd have to try python3.7, python3.8, python3.9, etc.
Re: [PATCH v9 0/6] Rework iotests/check
26.01.2021 18:36, Kevin Wolf wrote: Am 26.01.2021 um 16:15 hat Vladimir Sementsov-Ogievskiy geschrieben: OK, thanks for handling it! You're welcome. Only problem now: Max sent a conflicting pull request that touches 'group'. He suggested that we could split the deletion of 'group' from the 'check' rewrite and merge it only later when nobody touches 'group' any more (because it's unused). I think it's OK.. Nothing really wrong in forgetting to remove unused file, and remove it later :) The other option is that I wait a bit or speculatively merge his tree (with a lot more patches) before doing my pull request in the hope that it doesn't fail. When will we move to python 3.7? I seem to remember that 3.6 is used by more or less all of the current enterprise distributions, so I'm afraid it will be a while. Kevin -- Best regards, Vladimir
Re: [PATCH v9 0/6] Rework iotests/check
Am 26.01.2021 um 16:15 hat Vladimir Sementsov-Ogievskiy geschrieben: > OK, thanks for handling it! You're welcome. Only problem now: Max sent a conflicting pull request that touches 'group'. He suggested that we could split the deletion of 'group' from the 'check' rewrite and merge it only later when nobody touches 'group' any more (because it's unused). The other option is that I wait a bit or speculatively merge his tree (with a lot more patches) before doing my pull request in the hope that it doesn't fail. > When will we move to python 3.7? I seem to remember that 3.6 is used by more or less all of the current enterprise distributions, so I'm afraid it will be a while. Kevin
Re: [PATCH v9 0/6] Rework iotests/check
Am 26.01.2021 um 14:19 hat Vladimir Sementsov-Ogievskiy geschrieben: > 26.01.2021 15:53, Kevin Wolf wrote: > > Am 25.01.2021 um 19:50 hat Vladimir Sementsov-Ogievskiy geschrieben: > > > v9: > > > 01: new, one more whitespace-fixing patch > > > testenv: allow case when we don't have system-arch emulator, but have > > > several for another architectures > > > change direct os.access(..., os.X_OK) calls to new helper > > > function which also check that path is a file > > > testrunner: s/fail/not run/ for 'No qualified output' > > > drop elapsed time arg for one of 'not run' results (now no > > > elapsed time for any 'not run' result) > > > > More CI fun: > > > > Traceback (most recent call last): > >File "./check", line 117, in > > testfinder = TestFinder(test_dir=env.source_iotests) > >File "/builds/.../qemu/tests/qemu-iotests/findtests.py", line 53, in > > __init__ > > for line in f: > >File "/usr/lib64/python3.6/encodings/ascii.py", line 26, in decode > > return codecs.ascii_decode(input, self.errors)[0] > > UnicodeDecodeError: 'ascii' codec can't decode byte 0xe2 in position 1591: > > ordinal not in range(128) > > Can be solved by adding encoding='utf8' to the open().. But.. Adding it > everywhere is not fun. > > So, what about just define PYTHONUTF8=1 for running check in check-block.sh? > > https://docs.python.org/3/using/cmdline.html#envvar-PYTHONUTF8 That looked nice, but we both missed the important line: "New in version 3.7: See PEP 540 for more details." So I'm back to explicitly requesting utf-8 encoding everywhere and that seems to finally make it pass in the CI. Kevin diff --git a/tests/qemu-iotests/findtests.py b/tests/qemu-iotests/findtests.py index d0c72efd6a..dd77b453b8 100644 --- a/tests/qemu-iotests/findtests.py +++ b/tests/qemu-iotests/findtests.py @@ -49,7 +49,7 @@ class TestFinder: os.path.isfile(f + '.out')] for t in self.all_tests: -with open(t) as f: +with open(t, encoding="utf-8") as f: for line in f: if line.startswith('# group: '): for g in line.split()[2:]: @@ -57,7 +57,7 @@ class TestFinder: break def add_group_file(self, fname: str) -> None: -with open(fname) as f: +with open(fname, encoding="utf-8") as f: for line in f: line = line.strip() diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py index 046f9ce38f..a581be6a29 100644 --- a/tests/qemu-iotests/testrunner.py +++ b/tests/qemu-iotests/testrunner.py @@ -41,7 +41,8 @@ def silent_unlink(path: Path) -> None: def file_diff(file1: str, file2: str) -> List[str]: -with open(file1) as f1, open(file2) as f2: +with open(file1, encoding="utf-8") as f1, \ + open(file2, encoding="utf-8") as f2: # We want to ignore spaces at line ends. There are a lot of mess about # it in iotests. # TODO: fix all tests to not produce extra spaces, fix all .out files @@ -81,7 +82,7 @@ class LastElapsedTime(ContextManager['LastElapsedTime']): self.cache: Dict[str, Dict[str, Dict[str, float]]] try: -with open(cache_file) as f: +with open(cache_file, encoding="utf-8") as f: self.cache = json.load(f) except (OSError, ValueError): self.cache = {} @@ -102,7 +103,7 @@ class LastElapsedTime(ContextManager['LastElapsedTime']): d.setdefault(self.env.imgproto, {})[self.env.imgfmt] = elapsed def save(self) -> None: -with open(self.cache_file, 'w') as f: +with open(self.cache_file, 'w', encoding="utf-8") as f: json.dump(self.cache, f) def __enter__(self) -> 'LastElapsedTime': @@ -245,7 +246,7 @@ class TestRunner(ContextManager['TestRunner']): if self.env.debug: args.append('-d') -with f_test.open() as f: +with f_test.open(encoding="utf-8") as f: try: if f.readline() == '#!/usr/bin/env python3': args.insert(0, self.env.python) @@ -256,7 +257,7 @@ class TestRunner(ContextManager['TestRunner']): env.update(self.test_run_env) t0 = time.time() -with f_bad.open('w') as f: +with f_bad.open('w', encoding="utf-8") as f: proc = subprocess.Popen(args, cwd=str(f_test.parent), env=env, stdout=f, stderr=subprocess.STDOUT) try:
Re: [PATCH v9 0/6] Rework iotests/check
26.01.2021 18:07, Kevin Wolf wrote: Am 26.01.2021 um 14:19 hat Vladimir Sementsov-Ogievskiy geschrieben: 26.01.2021 15:53, Kevin Wolf wrote: Am 25.01.2021 um 19:50 hat Vladimir Sementsov-Ogievskiy geschrieben: v9: 01: new, one more whitespace-fixing patch testenv: allow case when we don't have system-arch emulator, but have several for another architectures change direct os.access(..., os.X_OK) calls to new helper function which also check that path is a file testrunner: s/fail/not run/ for 'No qualified output' drop elapsed time arg for one of 'not run' results (now no elapsed time for any 'not run' result) More CI fun: Traceback (most recent call last): File "./check", line 117, in testfinder = TestFinder(test_dir=env.source_iotests) File "/builds/.../qemu/tests/qemu-iotests/findtests.py", line 53, in __init__ for line in f: File "/usr/lib64/python3.6/encodings/ascii.py", line 26, in decode return codecs.ascii_decode(input, self.errors)[0] UnicodeDecodeError: 'ascii' codec can't decode byte 0xe2 in position 1591: ordinal not in range(128) Can be solved by adding encoding='utf8' to the open().. But.. Adding it everywhere is not fun. So, what about just define PYTHONUTF8=1 for running check in check-block.sh? https://docs.python.org/3/using/cmdline.html#envvar-PYTHONUTF8 That looked nice, but we both missed the important line: "New in version 3.7: See PEP 540 for more details." So I'm back to explicitly requesting utf-8 encoding everywhere and that seems to finally make it pass in the CI. Kevin diff --git a/tests/qemu-iotests/findtests.py b/tests/qemu-iotests/findtests.py index d0c72efd6a..dd77b453b8 100644 --- a/tests/qemu-iotests/findtests.py +++ b/tests/qemu-iotests/findtests.py @@ -49,7 +49,7 @@ class TestFinder: os.path.isfile(f + '.out')] for t in self.all_tests: -with open(t) as f: +with open(t, encoding="utf-8") as f: for line in f: if line.startswith('# group: '): for g in line.split()[2:]: @@ -57,7 +57,7 @@ class TestFinder: break def add_group_file(self, fname: str) -> None: -with open(fname) as f: +with open(fname, encoding="utf-8") as f: for line in f: line = line.strip() diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py index 046f9ce38f..a581be6a29 100644 --- a/tests/qemu-iotests/testrunner.py +++ b/tests/qemu-iotests/testrunner.py @@ -41,7 +41,8 @@ def silent_unlink(path: Path) -> None: def file_diff(file1: str, file2: str) -> List[str]: -with open(file1) as f1, open(file2) as f2: +with open(file1, encoding="utf-8") as f1, \ + open(file2, encoding="utf-8") as f2: # We want to ignore spaces at line ends. There are a lot of mess about # it in iotests. # TODO: fix all tests to not produce extra spaces, fix all .out files @@ -81,7 +82,7 @@ class LastElapsedTime(ContextManager['LastElapsedTime']): self.cache: Dict[str, Dict[str, Dict[str, float]]] try: -with open(cache_file) as f: +with open(cache_file, encoding="utf-8") as f: self.cache = json.load(f) except (OSError, ValueError): self.cache = {} @@ -102,7 +103,7 @@ class LastElapsedTime(ContextManager['LastElapsedTime']): d.setdefault(self.env.imgproto, {})[self.env.imgfmt] = elapsed def save(self) -> None: -with open(self.cache_file, 'w') as f: +with open(self.cache_file, 'w', encoding="utf-8") as f: json.dump(self.cache, f) def __enter__(self) -> 'LastElapsedTime': @@ -245,7 +246,7 @@ class TestRunner(ContextManager['TestRunner']): if self.env.debug: args.append('-d') -with f_test.open() as f: +with f_test.open(encoding="utf-8") as f: try: if f.readline() == '#!/usr/bin/env python3': args.insert(0, self.env.python) @@ -256,7 +257,7 @@ class TestRunner(ContextManager['TestRunner']): env.update(self.test_run_env) t0 = time.time() -with f_bad.open('w') as f: +with f_bad.open('w', encoding="utf-8") as f: proc = subprocess.Popen(args, cwd=str(f_test.parent), env=env, stdout=f, stderr=subprocess.STDOUT) try: OK, thanks for handling it! When will we move to python 3.7? -- Best regards, Vladimir
Re: [PATCH v9 0/6] Rework iotests/check
Am 26.01.2021 um 14:19 hat Vladimir Sementsov-Ogievskiy geschrieben: > 26.01.2021 15:53, Kevin Wolf wrote: > > Am 25.01.2021 um 19:50 hat Vladimir Sementsov-Ogievskiy geschrieben: > > > v9: > > > 01: new, one more whitespace-fixing patch > > > testenv: allow case when we don't have system-arch emulator, but have > > > several for another architectures > > > change direct os.access(..., os.X_OK) calls to new helper > > > function which also check that path is a file > > > testrunner: s/fail/not run/ for 'No qualified output' > > > drop elapsed time arg for one of 'not run' results (now no > > > elapsed time for any 'not run' result) > > > > More CI fun: > > > > Traceback (most recent call last): > >File "./check", line 117, in > > testfinder = TestFinder(test_dir=env.source_iotests) > >File "/builds/.../qemu/tests/qemu-iotests/findtests.py", line 53, in > > __init__ > > for line in f: > >File "/usr/lib64/python3.6/encodings/ascii.py", line 26, in decode > > return codecs.ascii_decode(input, self.errors)[0] > > UnicodeDecodeError: 'ascii' codec can't decode byte 0xe2 in position 1591: > > ordinal not in range(128) > > Can be solved by adding encoding='utf8' to the open().. But.. Adding > it everywhere is not fun. > > So, what about just define PYTHONUTF8=1 for running check in > check-block.sh? Ah, I didn't know this one. Yes, that's probably nicer than my attempt of adding an explicit encoding everywhere. > > Traceback (most recent call last): > >File "./check", line 112, in > > env = TestEnv(imgfmt=args.imgfmt, imgproto=args.imgproto, > >File "/builds/.../qemu/tests/qemu-iotests/testenv.py", line 216, in > > __init__ > > self.qemu_default_machine = get_default_machine(self.qemu_prog) > >File "/builds/.../qemu/tests/qemu-iotests/testenv.py", line 41, in > > get_default_machine > > default_machine = next(m for m in machines if m.endswith(' (default)')) > > Looking at original check, default_machine should be empty string in this > case. > > so we need > > diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py > index 8b80425670..222395caef 100644 > --- a/tests/qemu-iotests/testenv.py > +++ b/tests/qemu-iotests/testenv.py > @@ -38,7 +38,11 @@ def get_default_machine(qemu_prog: str) -> str: >stdout=subprocess.PIPE).stdout > machines = outp.split('\n') > -default_machine = next(m for m in machines if m.endswith(' (default)')) > +try: > +default_machine = next(m for m in machines if m.endswith(' > (default)')) > +except StopIteration: > +return '' > + > default_machine = default_machine.split(' ', 1)[0] > alias_suf = ' (alias of {})'.format(default_machine) This one looks a bit nicer than mine (using None instead of an exception), too, so I'll apply this one as well for the next test cycle. Kevin
Re: [PATCH v9 0/6] Rework iotests/check
26.01.2021 15:53, Kevin Wolf wrote: Am 25.01.2021 um 19:50 hat Vladimir Sementsov-Ogievskiy geschrieben: v9: 01: new, one more whitespace-fixing patch testenv: allow case when we don't have system-arch emulator, but have several for another architectures change direct os.access(..., os.X_OK) calls to new helper function which also check that path is a file testrunner: s/fail/not run/ for 'No qualified output' drop elapsed time arg for one of 'not run' results (now no elapsed time for any 'not run' result) More CI fun: Traceback (most recent call last): File "./check", line 117, in testfinder = TestFinder(test_dir=env.source_iotests) File "/builds/.../qemu/tests/qemu-iotests/findtests.py", line 53, in __init__ for line in f: File "/usr/lib64/python3.6/encodings/ascii.py", line 26, in decode return codecs.ascii_decode(input, self.errors)[0] UnicodeDecodeError: 'ascii' codec can't decode byte 0xe2 in position 1591: ordinal not in range(128) Can be solved by adding encoding='utf8' to the open().. But.. Adding it everywhere is not fun. So, what about just define PYTHONUTF8=1 for running check in check-block.sh? https://docs.python.org/3/using/cmdline.html#envvar-PYTHONUTF8 diff --git a/tests/check-block.sh b/tests/check-block.sh index ac32fd67dd..9dcadc435f 100755 --- a/tests/check-block.sh +++ b/tests/check-block.sh @@ -74,6 +74,8 @@ cd tests/qemu-iotests # QEMU_CHECK_BLOCK_AUTO is used to disable some unstable sub-tests export QEMU_CHECK_BLOCK_AUTO=1 +export PYTHONUTF8=1 + ret=0 for fmt in $format_list ; do ${PYTHON} ./check -makecheck -$fmt $group || ret=1 Traceback (most recent call last): File "./check", line 112, in env = TestEnv(imgfmt=args.imgfmt, imgproto=args.imgproto, File "/builds/.../qemu/tests/qemu-iotests/testenv.py", line 216, in __init__ self.qemu_default_machine = get_default_machine(self.qemu_prog) File "/builds/.../qemu/tests/qemu-iotests/testenv.py", line 41, in get_default_machine default_machine = next(m for m in machines if m.endswith(' (default)')) Looking at original check, default_machine should be empty string in this case. so we need diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py index 8b80425670..222395caef 100644 --- a/tests/qemu-iotests/testenv.py +++ b/tests/qemu-iotests/testenv.py @@ -38,7 +38,11 @@ def get_default_machine(qemu_prog: str) -> str: stdout=subprocess.PIPE).stdout machines = outp.split('\n') -default_machine = next(m for m in machines if m.endswith(' (default)')) +try: +default_machine = next(m for m in machines if m.endswith(' (default)')) +except StopIteration: +return '' + default_machine = default_machine.split(' ', 1)[0] alias_suf = ' (alias of {})'.format(default_machine) -- Best regards, Vladimir
Re: [PATCH v9 0/6] Rework iotests/check
Am 25.01.2021 um 19:50 hat Vladimir Sementsov-Ogievskiy geschrieben: > v9: > 01: new, one more whitespace-fixing patch > testenv: allow case when we don't have system-arch emulator, but have several > for another architectures > change direct os.access(..., os.X_OK) calls to new helper function > which also check that path is a file > testrunner: s/fail/not run/ for 'No qualified output' > drop elapsed time arg for one of 'not run' results (now no > elapsed time for any 'not run' result) More CI fun: Traceback (most recent call last): File "./check", line 117, in testfinder = TestFinder(test_dir=env.source_iotests) File "/builds/.../qemu/tests/qemu-iotests/findtests.py", line 53, in __init__ for line in f: File "/usr/lib64/python3.6/encodings/ascii.py", line 26, in decode return codecs.ascii_decode(input, self.errors)[0] UnicodeDecodeError: 'ascii' codec can't decode byte 0xe2 in position 1591: ordinal not in range(128) Traceback (most recent call last): File "./check", line 112, in env = TestEnv(imgfmt=args.imgfmt, imgproto=args.imgproto, File "/builds/.../qemu/tests/qemu-iotests/testenv.py", line 216, in __init__ self.qemu_default_machine = get_default_machine(self.qemu_prog) File "/builds/.../qemu/tests/qemu-iotests/testenv.py", line 41, in get_default_machine default_machine = next(m for m in machines if m.endswith(' (default)')) StopIteration Kevin
[PATCH v9 0/6] Rework iotests/check
v9: 01: new, one more whitespace-fixing patch testenv: allow case when we don't have system-arch emulator, but have several for another architectures change direct os.access(..., os.X_OK) calls to new helper function which also check that path is a file testrunner: s/fail/not run/ for 'No qualified output' drop elapsed time arg for one of 'not run' results (now no elapsed time for any 'not run' result) Vladimir Sementsov-Ogievskiy (6): iotests: 146: drop extra whitespaces from .out file iotests: add findtests.py iotests: add testenv.py iotests: add testrunner.py iotests: rewrite check into python iotests: rename and move 169 and 199 tests docs/devel/testing.rst| 50 +- Makefile |1 - tests/check-block.sh |2 +- tests/qemu-iotests/146.out| 780 ++-- tests/qemu-iotests/check | 1095 ++--- tests/qemu-iotests/common.env.in |3 - tests/qemu-iotests/findtests.py | 159 +++ tests/qemu-iotests/group | 321 - tests/qemu-iotests/iotests.py |8 + tests/qemu-iotests/meson.build|3 - tests/qemu-iotests/testenv.py | 279 + tests/qemu-iotests/testrunner.py | 366 ++ .../migrate-bitmaps-postcopy-test}|0 .../migrate-bitmaps-postcopy-test.out}|0 .../{169 => tests/migrate-bitmaps-test} |0 .../migrate-bitmaps-test.out} |0 16 files changed, 1381 insertions(+), 1686 deletions(-) delete mode 100644 tests/qemu-iotests/common.env.in create mode 100644 tests/qemu-iotests/findtests.py delete mode 100644 tests/qemu-iotests/group create mode 100644 tests/qemu-iotests/testenv.py create mode 100644 tests/qemu-iotests/testrunner.py rename tests/qemu-iotests/{199 => tests/migrate-bitmaps-postcopy-test} (100%) rename tests/qemu-iotests/{199.out => tests/migrate-bitmaps-postcopy-test.out} (100%) rename tests/qemu-iotests/{169 => tests/migrate-bitmaps-test} (100%) rename tests/qemu-iotests/{169.out => tests/migrate-bitmaps-test.out} (100%) -- 2.29.2