[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v7 09/11] iotests: add testrunner.py
From: |
Eric Blake |
Subject: |
Re: [PATCH v7 09/11] iotests: add testrunner.py |
Date: |
Thu, 21 Jan 2021 11:02:56 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 |
On 1/16/21 7:44 AM, Vladimir Sementsov-Ogievskiy wrote:
> Add TestRunner class, which will run tests in a new python iotests
> running framework.
>
> There are some differences with current ./check behavior, most
> significant are:
> - Consider all tests self-executable, just run them, don't run python
> by hand.
> - Elapsed time is cached in json file
> - Elapsed time precision increased a bit
> - use python difflib instead of "diff -w", to ignore spaces at line
> ends strip lines by hand. Do not ignore other spaces.
Awkward wording. Maybe:
Instead of using "diff -w" which ignores all whitespace differences,
manually strip whitespace at line end then use python difflib, which no
longer ignores spacing mid-line
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> tests/qemu-iotests/testrunner.py | 344 +++++++++++++++++++++++++++++++
> 1 file changed, 344 insertions(+)
> create mode 100644 tests/qemu-iotests/testrunner.py
>
> diff --git a/tests/qemu-iotests/testrunner.py
> b/tests/qemu-iotests/testrunner.py
> new file mode 100644
> index 0000000000..92722cc68b
> --- /dev/null
> +++ b/tests/qemu-iotests/testrunner.py
> @@ -0,0 +1,344 @@
> +# Class for actual tests running.
for actually running tests
again, should this file be 755 with #! python?
> +
> +def file_diff(file1: str, file2: str) -> List[str]:
> + with open(file1) as f1, open(file2) 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
> + # and use strict diff here!
> + seq1 = [line.rstrip() for line in f1]
> + seq2 = [line.rstrip() for line in f2]
> + return list(difflib.unified_diff(seq1, seq2, file1, file2))
Offhand, do you have the list of tests where (actual/expected) output
has trailing whitespace and would fail without the .rstrip()?
> +
> +
> +# We want to save current tty settings during test run,
> +# since an aborting qemu call may leave things screwed up.
> +@contextmanager
> +def savetty() -> Iterator[None]:
> + isterm = sys.stdin.isatty()
> + if isterm:
> + fd = sys.stdin.fileno()
> + attr = termios.tcgetattr(0)
Isn't fd always going to be 0? It looks odd to hard-code zero in the
very next line; either we should s/0/fd/ here, or...
> +
> + try:
> + yield
> + finally:
> + if isterm:
> + termios.tcsetattr(fd, termios.TCSADRAIN, attr)
... s/fd/0/ here and drop fd altogether.
> +
> +
> +class LastElapsedTime(AbstractContextManager['LastElapsedTime']):
> + """ Cache for elapsed time for tests, to show it during new test run
> +
> + Use get() in any time. But, if use update you should then call save(),
> + or use update() inside with-block.
Grammar is hard, maybe:
It is safe to use get() at any time. To use update(), you must either
use a with-block or first use save().
> + def test_print_one_line(self, test: str, starttime: str,
> +
> + if status == 'pass':
> + col = '\033[32m'
> + elif status == 'fail':
> + col = '\033[1m\033[31m'
> + elif status == 'not run':
> + col = '\033[33m'
This hard-codes the use of ANSI escape sequences without first checking
that we are writing to a terminal. Is that wise? Should we have this
be tunable by a tri-state command-line option, similar to ls --color?
(--color=auto is default, and bases decision on istty(), --color=off
turns color off even for a terminal, --color=on uses color even when
outputting to a pipe, which can be useful depending on the other end of
the pipeline...)
> + with f_test.open() as f:
> + try:
> + if f.readline() == '#!/usr/bin/env python3':
> + args.insert(0, self.env.python)
> + except UnicodeDecodeError: # binary test? for future.
> + pass
Is pass the right action here? Or will it silently skip a test file
with encoding errors?
Again, I'm not comfortable enough to give a full review of the python,
but it looks fairly similar to the existing shell code, and with the
series applied, things still work. So I can offer
Tested-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
- Re: [PATCH v7 07/11] iotests: add findtests.py, (continued)
Re: [PATCH v7 07/11] iotests: add findtests.py, Kevin Wolf, 2021/01/22
[PATCH v7 08/11] iotests: add testenv.py, Vladimir Sementsov-Ogievskiy, 2021/01/16
[PATCH v7 09/11] iotests: add testrunner.py, Vladimir Sementsov-Ogievskiy, 2021/01/16
[PATCH v7 10/11] iotests: rewrite check into python, Vladimir Sementsov-Ogievskiy, 2021/01/16