From: Kevin Wolf <kwolf@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, mreitz@redhat.com,
den@openvz.org, jsnow@redhat.com
Subject: Re: [PATCH v3 06/10] iotests: add testfinder.py
Date: Fri, 8 May 2020 10:49:03 +0200 [thread overview]
Message-ID: <20200508084903.GA4970@linux.fritz.box> (raw)
In-Reply-To: <5a185a4e-1c5d-9887-c88c-6ca71ebeeee1@virtuozzo.com>
Am 07.05.2020 um 19:43 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 21.04.2020 19:56, Kevin Wolf wrote:
> > Am 21.04.2020 um 09:35 hat Vladimir Sementsov-Ogievskiy geschrieben:
> > > Add python script with new logic of searching for tests:
> > >
> > > Current ./check behavior:
> > > - tests are named [0-9][0-9][0-9]
> > > - tests must be registered in group file (even if test doesn't belong
> > > to any group, like 142)
> > >
> > > Behavior of new test:
> > > - group file is dropped
> > > - tests are searched by file-name instead of group file, so it's not
> > > needed more to "register the test", just create it with name
> > > *-test. Old names like [0-9][0-9][0-9] are supported too, but not
> > > recommended for new tests
> >
> > I wonder if a tests/ subdirectory instead of the -test suffix would
> > organise things a bit better.
> >
>
> It will make more difficult to import iotests.py.. Calling common.rc from
> bash tests will need to be modified too.
>
> So, we'll need additional line in all python tests:
>
> sys.path.append(os.path.join(os.path.dirname(__file__), '..'))
Hm, yes, this is nasty.
Would it be any better have a wrapper script that just imports the
actual test so that the import path would still contain the main
qemu-iotests/ directory?
> which doesn't seem to be good practice.. So, instead we'd better call
> tests with PYTHONPATH set appropriately..
This is another option, especially if we do want to create a lib/.
> and modify bash tests to do
> . ../common.rc
> . ../common.filter
> <etc>
>
> or again, better to export BASH_TEST_LIB directory.
I don't think this is necessary because the working directory wouldn't
change, so bash scripts should just keep working as before.
> Is it worth doing?
>
> I think, there are two variants:
>
> 1) keep as is: all in one directory, add *-test notation
I think it would make it rather hard to find the files that belong to
the test harness implementation between all the tests. Currently, 'ls'
in the qemu-iotests directory is still kind of usable because all the
test cases are at the start and everything that comes later is not a
test.
> 2) go further and restructure to something like:
>
> iotests/<only check script and its dependencies here>
> iotests/tests/<all-tests-here>
> iotests/lib/python/iotests.py
> iotests/lib/bash/<common.rc and friends here>
>
> And then, check script will export PYTHONPATH and BASH_TEST_LIB
> variables.
I think leaving everything except named tests where it is, but setting
PYTHONPATH or having a wrapper script, is still a third option that's
worth considering. It sounds like the most attrative option to me.
Kevin
next prev parent reply other threads:[~2020-05-08 8:50 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-21 7:35 [PATCH v3 00/10] Rework iotests/check Vladimir Sementsov-Ogievskiy
2020-04-21 7:35 ` [PATCH v3 01/10] iotests/277: use dot slash for nbd-fault-injector.py running Vladimir Sementsov-Ogievskiy
2020-04-21 12:54 ` Eric Blake
2020-04-21 13:04 ` Vladimir Sementsov-Ogievskiy
2020-04-21 7:35 ` [PATCH v3 02/10] iotests: fix some whitespaces in test output files Vladimir Sementsov-Ogievskiy
2020-04-21 7:35 ` [PATCH v3 03/10] iotests/283: make executable Vladimir Sementsov-Ogievskiy
2020-04-21 12:55 ` Eric Blake
2020-05-14 6:17 ` Philippe Mathieu-Daudé
2020-04-21 7:35 ` [PATCH v3 04/10] iotests/check: move QEMU_VXHS_PROG to common.rc Vladimir Sementsov-Ogievskiy
2020-04-21 16:03 ` Kevin Wolf
2020-04-22 5:14 ` Vladimir Sementsov-Ogievskiy
2020-04-21 7:35 ` [PATCH v3 05/10] iotests: define group in each iotest Vladimir Sementsov-Ogievskiy
2020-04-21 7:35 ` [PATCH v3 06/10] iotests: add testfinder.py Vladimir Sementsov-Ogievskiy
2020-04-21 16:56 ` Kevin Wolf
2020-04-22 5:35 ` Vladimir Sementsov-Ogievskiy
2020-04-22 11:53 ` Kevin Wolf
2020-04-22 12:49 ` Vladimir Sementsov-Ogievskiy
2020-04-22 13:06 ` Kevin Wolf
2020-05-07 17:43 ` Vladimir Sementsov-Ogievskiy
2020-05-08 8:49 ` Kevin Wolf [this message]
2020-05-08 9:42 ` Vladimir Sementsov-Ogievskiy
2020-05-13 21:58 ` John Snow
2020-05-14 4:54 ` Vladimir Sementsov-Ogievskiy
2020-05-14 5:06 ` John Snow
2020-05-14 5:31 ` Vladimir Sementsov-Ogievskiy
2020-04-21 7:35 ` [PATCH v3 07/10] iotests: add testenv.py Vladimir Sementsov-Ogievskiy
2020-04-21 7:35 ` [PATCH v3 08/10] iotests: add testrunner.py Vladimir Sementsov-Ogievskiy
2020-04-21 7:36 ` [PATCH v3 09/10] iotests: rewrite check into python Vladimir Sementsov-Ogievskiy
2020-04-21 7:40 ` Vladimir Sementsov-Ogievskiy
2020-04-21 7:36 ` [PATCH v3 10/10] iotests: rename 169 and 199 Vladimir Sementsov-Ogievskiy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200508084903.GA4970@linux.fritz.box \
--to=kwolf@redhat.com \
--cc=den@openvz.org \
--cc=jsnow@redhat.com \
--cc=mreitz@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=vsementsov@virtuozzo.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.