Re: [PATCH v2] iotests/check: move long options to double dash

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

ping.

Patch is reviewed)

03.09.2021 15:00, Vladimir Sementsov-Ogievskiy wrote:
> So, the change:
> 
>    -makecheck -> --makecheck
>    -gdb       -> --gdb
>    -valgrind  -> --valgrind
>    -misalign  -> --misalign
>    -nocache   -> --nocache
>    -qcow2 (and other formats) -> --qcow2
>    -file (and other protocols) -> --file
> 
> Motivation:
> 
> 1. check scripts uses ArgumentParser to parse options, which supports
>     combining of short options. So using one dash for long options is a
>     bit ambiguous.
> 
> 2. Several long options are already have double dash:
>    --dry-run, --color, --groups, --exclude-groups, --start-from
> 
> 3. -misalign is used to add --misalign option (two dashes) to qemu-io
> 
> 4. qemu-io and qemu-nbd has --nocache option (two dashes)
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> 
> v2: cover more things, update also format and protocol options
> 
>   docs/devel/testing.rst       | 12 ++++++------
>   .gitlab-ci.d/buildtest.yml   |  4 ++--
>   tests/check-block.sh         |  2 +-
>   tests/qemu-iotests/README    |  7 ++++---
>   tests/qemu-iotests/check     | 14 +++++++-------
>   tests/qemu-iotests/common.rc |  4 ++--
>   6 files changed, 22 insertions(+), 21 deletions(-)
> 
> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
> index 4a0abbf23d..907b18a600 100644
> --- a/docs/devel/testing.rst
> +++ b/docs/devel/testing.rst
> @@ -153,16 +153,16 @@ with arguments:
>   .. code::
>   
>     # test with qcow2 format
> -  ./check -qcow2
> +  ./check --qcow2
>     # or test a different protocol
> -  ./check -nbd
> +  ./check --nbd
>   
>   It's also possible to list test numbers explicitly:
>   
>   .. code::
>   
>     # run selected cases with qcow2 format
> -  ./check -qcow2 001 030 153
> +  ./check --qcow2 001 030 153
>   
>   Cache mode can be selected with the "-c" option, which may help reveal bugs
>   that are specific to certain cache mode.
> @@ -229,7 +229,7 @@ Debugging a test case
>   The following options to the ``check`` script can be useful when debugging
>   a failing test:
>   
> -* ``-gdb`` wraps every QEMU invocation in a ``gdbserver``, which waits for a
> +* ``--gdb`` wraps every QEMU invocation in a ``gdbserver``, which waits for a
>     connection from a gdb client.  The options given to ``gdbserver`` (e.g. the
>     address on which to listen for connections) are taken from the ``$GDB_OPTIONS``
>     environment variable.  By default (if ``$GDB_OPTIONS`` is empty), it listens on
> @@ -237,10 +237,10 @@ a failing test:
>     It is possible to connect to it for example with
>     ``gdb -iex "target remote $addr"``, where ``$addr`` is the address
>     ``gdbserver`` listens on.
> -  If the ``-gdb`` option is not used, ``$GDB_OPTIONS`` is ignored,
> +  If the ``--gdb`` option is not used, ``$GDB_OPTIONS`` is ignored,
>     regardless of whether it is set or not.
>   
> -* ``-valgrind`` attaches a valgrind instance to QEMU. If it detects
> +* ``--valgrind`` attaches a valgrind instance to QEMU. If it detects
>     warnings, it will print and save the log in
>     ``$TEST_DIR/<valgrind_pid>.valgrind``.
>     The final command line will be ``valgrind --log-file=$TEST_DIR/
> diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
> index e74998efb9..139c27554d 100644
> --- a/.gitlab-ci.d/buildtest.yml
> +++ b/.gitlab-ci.d/buildtest.yml
> @@ -303,10 +303,10 @@ build-tcg-disabled:
>       - make check-unit
>       - make check-qapi-schema
>       - cd tests/qemu-iotests/
> -    - ./check -raw 001 002 003 004 005 008 009 010 011 012 021 025 032 033 048
> +    - ./check --raw 001 002 003 004 005 008 009 010 011 012 021 025 032 033 048
>               052 063 077 086 101 104 106 113 148 150 151 152 157 159 160 163
>               170 171 183 184 192 194 208 221 226 227 236 253 277 image-fleecing
> -    - ./check -qcow2 028 051 056 057 058 065 068 082 085 091 095 096 102 122
> +    - ./check --qcow2 028 051 056 057 058 065 068 082 085 091 095 096 102 122
>               124 132 139 142 144 145 151 152 155 157 165 194 196 200 202
>               208 209 216 218 227 234 246 247 248 250 254 255 257 258
>               260 261 262 263 264 270 272 273 277 279 image-fleecing
> diff --git a/tests/check-block.sh b/tests/check-block.sh
> index f86cb863de..cff1263c0b 100755
> --- a/tests/check-block.sh
> +++ b/tests/check-block.sh
> @@ -77,7 +77,7 @@ export PYTHONUTF8=1
>   
>   ret=0
>   for fmt in $format_list ; do
> -    ${PYTHON} ./check -makecheck -$fmt $group || ret=1
> +    ${PYTHON} ./check --makecheck --$fmt $group || ret=1
>   done
>   
>   exit $ret
> diff --git a/tests/qemu-iotests/README b/tests/qemu-iotests/README
> index 6079b401ae..8e1f3e19c3 100644
> --- a/tests/qemu-iotests/README
> +++ b/tests/qemu-iotests/README
> @@ -10,9 +10,10 @@ but no actual block drivers like ide, scsi or virtio.
>   
>   * Usage
>   
> -Just run ./check to run all tests for the raw image format, or ./check
> --qcow2 to test the qcow2 image format.  The output of ./check -h explains
> -additional options to test further image formats or I/O methods.
> +Just run ./check to run all tests for the raw image format,
> +or ./check --qcow2 to test the qcow2 image format.  The output of
> +./check -h explains additional options to test further image formats
> +or I/O methods.
>   
>   * Feedback and patches
>   
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index da1bfb839e..5ca9f31950 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -32,20 +32,20 @@ def make_argparser() -> argparse.ArgumentParser:
>   
>       p.add_argument('-n', '--dry-run', action='store_true',
>                      help='show me, do not run tests')
> -    p.add_argument('-makecheck', action='store_true',
> +    p.add_argument('--makecheck', action='store_true',
>                      help='pretty print output for make check')
>   
>       p.add_argument('-d', dest='debug', action='store_true', help='debug')
>       p.add_argument('-p', dest='print', action='store_true',
>                   help='redirects qemu\'s stdout and stderr to the test output')
> -    p.add_argument('-gdb', action='store_true',
> +    p.add_argument('--gdb', action='store_true',
>                      help="start gdbserver with $GDB_OPTIONS options \
>                           ('localhost:12345' if $GDB_OPTIONS is empty)")
> -    p.add_argument('-valgrind', action='store_true',
> +    p.add_argument('--valgrind', action='store_true',
>                       help='use valgrind, sets VALGRIND_QEMU environment '
>                       'variable')
>   
> -    p.add_argument('-misalign', action='store_true',
> +    p.add_argument('--misalign', action='store_true',
>                      help='misalign memory allocations')
>       p.add_argument('--color', choices=['on', 'off', 'auto'],
>                      default='auto', help="use terminal colors. The default "
> @@ -55,7 +55,7 @@ def make_argparser() -> argparse.ArgumentParser:
>       mg = g_env.add_mutually_exclusive_group()
>       # We don't set default for cachemode, as we need to distinguish default
>       # from user input later.
> -    mg.add_argument('-nocache', dest='cachemode', action='store_const',
> +    mg.add_argument('--nocache', dest='cachemode', action='store_const',
>                       const='none', help='set cache mode "none" (O_DIRECT), '
>                       'sets CACHEMODE environment variable')
>       mg.add_argument('-c', dest='cachemode',
> @@ -74,7 +74,7 @@ def make_argparser() -> argparse.ArgumentParser:
>           'At most one choice is allowed, default is "raw"')
>       mg = g_fmt.add_mutually_exclusive_group()
>       for fmt in format_list:
> -        mg.add_argument('-' + fmt, dest='imgfmt', action='store_const',
> +        mg.add_argument('--' + fmt, dest='imgfmt', action='store_const',
>                           const=fmt, help=f'test {fmt}')
>   
>       protocol_list = ['file', 'rbd', 'nbd', 'ssh', 'nfs', 'fuse']
> @@ -84,7 +84,7 @@ def make_argparser() -> argparse.ArgumentParser:
>           'At most one choice is allowed, default is "file"')
>       mg = g_prt.add_mutually_exclusive_group()
>       for prt in protocol_list:
> -        mg.add_argument('-' + prt, dest='imgproto', action='store_const',
> +        mg.add_argument('--' + prt, dest='imgproto', action='store_const',
>                           const=prt, help=f'test {prt}')
>   
>       g_bash = p.add_argument_group('bash tests options',
> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
> index d8582454de..0817756814 100644
> --- a/tests/qemu-iotests/common.rc
> +++ b/tests/qemu-iotests/common.rc
> @@ -124,7 +124,7 @@ fi
>   
>   # Set the variables to the empty string to turn Valgrind off
>   # for specific processes, e.g.
> -# $ VALGRIND_QEMU_IO= ./check -qcow2 -valgrind 015
> +# $ VALGRIND_QEMU_IO= ./check --qcow2 --valgrind 015
>   
>   : ${VALGRIND_QEMU_VM=$VALGRIND_QEMU}
>   : ${VALGRIND_QEMU_IMG=$VALGRIND_QEMU}
> @@ -134,7 +134,7 @@ fi
>   
>   # The Valgrind own parameters may be set with
>   # its environment variable VALGRIND_OPTS, e.g.
> -# $ VALGRIND_OPTS="--leak-check=yes" ./check -qcow2 -valgrind 015
> +# $ VALGRIND_OPTS="--leak-check=yes" ./check --qcow2 --valgrind 015
>   
>   _qemu_proc_exec()
>   {
> 


-- 
Best regards,
Vladimir