Re: [Qemu-devel] [PATCH] qemu-iotests: add a "how to" to ./README

[Kevin Wolf]

Am 24.07.2017 um 16:28 hat Eric Blake geschrieben:
> On 07/21/2017 04:34 AM, Stefan Hajnoczi wrote:
> > There is not much getting started documentation for qemu-iotests.  This
> > patch explains how to create a new test and covers the overall testing
> > approach.
> > 
> > +2. Create the test file
> > +
> > +Copy an existing test (one that most closely resembles what you wish to 
> > test)
> > +to the new test number:
> > +
> > +  cp 001 
> 
> And mark it executable (chmod a+x )

If you copy an existing test, it's already executable.

Kevin


pgpXCjgp2RLuL.pgp
Description: PGP signature

Re: [Qemu-devel] [PATCH] qemu-iotests: add a "how to" to ./README

[Eric Blake]

On 07/24/2017 09:34 AM, Peter Maydell wrote:
> On 24 July 2017 at 15:28, Eric Blake  wrote:
>> On 07/21/2017 04:34 AM, Stefan Hajnoczi wrote:
>>> There is not much getting started documentation for qemu-iotests.  This
>>> patch explains how to create a new test and covers the overall testing
>>> approach.
>>>
>>> +2. Create the test file
>>> +
>>> +Copy an existing test (one that most closely resembles what you wish to 
>>> test)
>>> +to the new test number:
>>> +
>>> +  cp 001 
>>
>> And mark it executable (chmod a+x )
> 
> It should already be executable because the test file being
> copied (001 in this case) is executable, shouldn't it?

Oh, I see what happened.  Rather than dropping to shell 'cp', I copied
via emacs' "ctrl-x ctrl-w" (write-file) and creating a new file name,
and that does not preserve executable bit.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] qemu-iotests: add a "how to" to ./README

[Peter Maydell]

On 24 July 2017 at 15:28, Eric Blake  wrote:
> On 07/21/2017 04:34 AM, Stefan Hajnoczi wrote:
>> There is not much getting started documentation for qemu-iotests.  This
>> patch explains how to create a new test and covers the overall testing
>> approach.
>>
>> +2. Create the test file
>> +
>> +Copy an existing test (one that most closely resembles what you wish to 
>> test)
>> +to the new test number:
>> +
>> +  cp 001 
>
> And mark it executable (chmod a+x )

It should already be executable because the test file being
copied (001 in this case) is executable, shouldn't it?

thanks
-- PMM

Re: [Qemu-devel] [PATCH] qemu-iotests: add a "how to" to ./README

[Eric Blake]

On 07/21/2017 04:34 AM, Stefan Hajnoczi wrote:
> There is not much getting started documentation for qemu-iotests.  This
> patch explains how to create a new test and covers the overall testing
> approach.
> 
> +2. Create the test file
> +
> +Copy an existing test (one that most closely resembles what you wish to test)
> +to the new test number:
> +
> +  cp 001 

And mark it executable (chmod a+x )

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] qemu-iotests: add a "how to" to ./README

[Peter Maydell]

On 21 July 2017 at 10:34, Stefan Hajnoczi  wrote:
> There is not much getting started documentation for qemu-iotests.  This
> patch explains how to create a new test and covers the overall testing
> approach.
>
> Cc: Ishani Chugh 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  tests/qemu-iotests/README | 83 
> +++
>  1 file changed, 83 insertions(+)
>
> diff --git a/tests/qemu-iotests/README b/tests/qemu-iotests/README
> index 6079b40..8259b9f 100644
> --- a/tests/qemu-iotests/README
> +++ b/tests/qemu-iotests/README
> @@ -14,8 +14,91 @@ 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.
>
> +* Testing approach
> +
> +Each test is an executable file (usually a bash script) that is run by the
> +./check test harness.  Standard out and standard error are captured to an
> +output file.  If the output file differs from the "golden master" output file
> +for the test then it fails.

Should ./check be run from the source tree, or the build tree? The
existing README text doesn't say and I don't think your additions
do either.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] qemu-iotests: add a "how to" to ./README

[Stefan Hajnoczi]

On Sun, Jul 23, 2017 at 11:18:50AM -0300, Philippe Mathieu-Daudé wrote:
> On 07/21/2017 12:51 PM, Stefan Hajnoczi wrote:
> > On Fri, Jul 21, 2017 at 07:16:34AM -0500, Eric Blake wrote:
> > > On 07/21/2017 04:34 AM, Stefan Hajnoczi wrote:
> > > > There is not much getting started documentation for qemu-iotests.  This
> > > > patch explains how to create a new test and covers the overall testing
> > > > approach.
> > > > 
> > > > Cc: Ishani Chugh 
> > > > Signed-off-by: Stefan Hajnoczi 
> > > > ---
> > > 
> > > > +3. Assign groups to the test
> > > > +
> > > > +Add your test to the ./group file.  This file is the index of tests 
> > > > and assigns
> > > > +them to functional groups like "rw" for read-write tests.  Most tests 
> > > > belong to
> > > > +the "rw" and "auto" groups.  "auto" means the test runs when ./check 
> > > > is invoked
> > > > +without a -g argument.
> > > > +
> > > > +Consider adding your test to the "quick" group if it executes quickly 
> > > > (<1s).
> > > 
> > > We have several tests going up to 5s (and I have a patch pending to
> > > remove two tests that took longer) - I think 1s is a bit on the short
> > > end for still classifying a test as quick.
> > 
> > I'm happy to accept any number blessed by Kevin.  I do think that 1
> > second is a safe maximum and no one should get in trouble for adding a
> > test that takes 1 second to the "quick" group.
> > 
> > > > +This group is run by "make check-block" and is often included as part 
> > > > of build
> > > > +tests in continuous integration systems.
> > > 
> > > It would still be nice to have 'make check' run 'make check-block'...
> > > but that's independent of this patch.
> > 
> > Yes, there is a separate discussion about that on the list right now.
> > Hopefully it will be added back.
> > 
> > > > +Once you are happy with the test output it can be used as the golden 
> > > > master
> > > > +with "mv .out.bad .out".  Rerun the test to 
> > > > verify
> > > > +that it passes.
> > > > +
> > > > +Congratulations, you've created a new test!
> > > 
> > > Maybe a reminder to 'git add' the new files, then submit the patch?
> > 
> > I thought about this too but decided not to get into the git and patch
> > submission business.  I carefully worded it to be about "creating" tests
> > rather than "adding" them to qemu.git because I wanted to limit the
> > scope of this README :).
> 
> Maybe something tiny like:
> 
>   Congratulations, you've created a new test!
> 
>   To share your test to upstream QEMU (highly recommended!) just follow
>   these recommendations: http://wiki.qemu.org/Contribute/SubmitAPatch

Sounds good.

Kevin: Any thoughts on this patch?

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] qemu-iotests: add a "how to" to ./README

[Kevin Wolf]

Am 21.07.2017 um 11:34 hat Stefan Hajnoczi geschrieben:
> There is not much getting started documentation for qemu-iotests.  This
> patch explains how to create a new test and covers the overall testing
> approach.
> 
> Cc: Ishani Chugh 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  tests/qemu-iotests/README | 83 
> +++
>  1 file changed, 83 insertions(+)
> 
> diff --git a/tests/qemu-iotests/README b/tests/qemu-iotests/README
> index 6079b40..8259b9f 100644
> --- a/tests/qemu-iotests/README
> +++ b/tests/qemu-iotests/README
> @@ -14,8 +14,91 @@ 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.
>  
> +* Testing approach
> +
> +Each test is an executable file (usually a bash script) that is run by the
> +./check test harness.  Standard out and standard error are captured to an
> +output file.  If the output file differs from the "golden master" output file
> +for the test then it fails.
> +
> +Tests are simply a sequence of commands that produce output and the test 
> itself
> +does not judge whether it passed or failed.  If you find yourself writing
> +checks to determine success or failure then you should rethink the test and
> +rely on output diffing instead.
> +
> +** Filtering volatile output
> +
> +When output contains absolute file paths, timestamps, process IDs, hostnames,
> +or other volatile strings, the diff against golden master output will fail.
> +Such output must be filtered to replace volatile strings with fixed
> +placeholders.
> +
> +For example, the path to the temporary working directory changes between test
> +runs so it must be filtered:
> +
> +  sed -e "s#$TEST_DIR/#TEST_DIR/#g"
> +
> +Commonly needed filters are available in ./common.filter.
> +
> +** Python tests
> +
> +Most tests are implemented in bash but it is difficult to interact with the 
> QMP
> +monitor.  A Python module called 'iotests' is available for tests that 
> require
> +JSON and interacting with QEMU.
> +
> +* How to create a test
> +
> +1. Choose an unused test number
> +
> +Tests are identified by a unique number.  Look for the highest test case 
> number
> +by looking at the test files.  Then search the qemu-devel@nongnu.org mailing
> +list to check if anyone has already sent patches using the next available
> +number.  You may need to increment the number a few times to reach an unused
> +number.
> +
> +2. Create the test file
> +
> +Copy an existing test (one that most closely resembles what you wish to test)
> +to the new test number:
> +
> +  cp 001 
> +
> +3. Assign groups to the test
> +
> +Add your test to the ./group file.  This file is the index of tests and 
> assigns
> +them to functional groups like "rw" for read-write tests.  Most tests belong 
> to
> +the "rw" and "auto" groups.  "auto" means the test runs when ./check is 
> invoked
> +without a -g argument.
> +
> +Consider adding your test to the "quick" group if it executes quickly (<1s).
> +This group is run by "make check-block" and is often included as part of 
> build
> +tests in continuous integration systems.
> +
> +4. Write the test
> +
> +Edit the test script.  Look at existing tests for examples.
> +
> +5. Generate the golden master file
> +
> +Run your test with "./check ".  You may need to pass additional
> +options to use an image format or protocol.

./check refuses to even run a test if the reference output is missing.
So in practice you need a 'touch .out' first.

> +The test will fail because there is no golden master yet.  Inspect the output
> +that your test generated with "cat .out.bad".
> +
> +Verify that the output is as expected and contains no volatile strings like
> +timestamps.  You may need to add filters to your test to remove volatile
> +strings.
> +
> +Once you are happy with the test output it can be used as the golden master
> +with "mv .out.bad .out".  Rerun the test to verify
> +that it passes.
> +
> +Congratulations, you've created a new test!

Looks good otherwise.

Kevin

Re: [Qemu-devel] [PATCH] qemu-iotests: add a "how to" to ./README

[Philippe Mathieu-Daudé]

On 07/21/2017 12:51 PM, Stefan Hajnoczi wrote:

On Fri, Jul 21, 2017 at 07:16:34AM -0500, Eric Blake wrote:

On 07/21/2017 04:34 AM, Stefan Hajnoczi wrote:

There is not much getting started documentation for qemu-iotests.  This
patch explains how to create a new test and covers the overall testing
approach.

Cc: Ishani Chugh 
Signed-off-by: Stefan Hajnoczi 
---



+3. Assign groups to the test
+
+Add your test to the ./group file.  This file is the index of tests and assigns
+them to functional groups like "rw" for read-write tests.  Most tests belong to
+the "rw" and "auto" groups.  "auto" means the test runs when ./check is invoked
+without a -g argument.
+
+Consider adding your test to the "quick" group if it executes quickly (<1s).


We have several tests going up to 5s (and I have a patch pending to
remove two tests that took longer) - I think 1s is a bit on the short
end for still classifying a test as quick.


I'm happy to accept any number blessed by Kevin.  I do think that 1
second is a safe maximum and no one should get in trouble for adding a
test that takes 1 second to the "quick" group.


+This group is run by "make check-block" and is often included as part of build
+tests in continuous integration systems.


It would still be nice to have 'make check' run 'make check-block'...
but that's independent of this patch.


Yes, there is a separate discussion about that on the list right now.
Hopefully it will be added back.


+Once you are happy with the test output it can be used as the golden master
+with "mv .out.bad .out".  Rerun the test to verify
+that it passes.
+
+Congratulations, you've created a new test!


Maybe a reminder to 'git add' the new files, then submit the patch?


I thought about this too but decided not to get into the git and patch
submission business.  I carefully worded it to be about "creating" tests
rather than "adding" them to qemu.git because I wanted to limit the
scope of this README :).


Maybe something tiny like:

  Congratulations, you've created a new test!

  To share your test to upstream QEMU (highly recommended!) just follow
  these recommendations: http://wiki.qemu.org/Contribute/SubmitAPatch



Stefan



Reviewed-by: Philippe Mathieu-Daudé 

Re: [Qemu-devel] [PATCH] qemu-iotests: add a "how to" to ./README

[Stefan Hajnoczi]

On Fri, Jul 21, 2017 at 07:16:34AM -0500, Eric Blake wrote:
> On 07/21/2017 04:34 AM, Stefan Hajnoczi wrote:
> > There is not much getting started documentation for qemu-iotests.  This
> > patch explains how to create a new test and covers the overall testing
> > approach.
> > 
> > Cc: Ishani Chugh 
> > Signed-off-by: Stefan Hajnoczi 
> > ---
> 
> > +3. Assign groups to the test
> > +
> > +Add your test to the ./group file.  This file is the index of tests and 
> > assigns
> > +them to functional groups like "rw" for read-write tests.  Most tests 
> > belong to
> > +the "rw" and "auto" groups.  "auto" means the test runs when ./check is 
> > invoked
> > +without a -g argument.
> > +
> > +Consider adding your test to the "quick" group if it executes quickly 
> > (<1s).
> 
> We have several tests going up to 5s (and I have a patch pending to
> remove two tests that took longer) - I think 1s is a bit on the short
> end for still classifying a test as quick.

I'm happy to accept any number blessed by Kevin.  I do think that 1
second is a safe maximum and no one should get in trouble for adding a
test that takes 1 second to the "quick" group.

> > +This group is run by "make check-block" and is often included as part of 
> > build
> > +tests in continuous integration systems.
> 
> It would still be nice to have 'make check' run 'make check-block'...
> but that's independent of this patch.

Yes, there is a separate discussion about that on the list right now.
Hopefully it will be added back.

> > +Once you are happy with the test output it can be used as the golden master
> > +with "mv .out.bad .out".  Rerun the test to 
> > verify
> > +that it passes.
> > +
> > +Congratulations, you've created a new test!
> 
> Maybe a reminder to 'git add' the new files, then submit the patch?

I thought about this too but decided not to get into the git and patch
submission business.  I carefully worded it to be about "creating" tests
rather than "adding" them to qemu.git because I wanted to limit the
scope of this README :).

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] qemu-iotests: add a "how to" to ./README

[Eric Blake]

On 07/21/2017 04:34 AM, Stefan Hajnoczi wrote:
> There is not much getting started documentation for qemu-iotests.  This
> patch explains how to create a new test and covers the overall testing
> approach.
> 
> Cc: Ishani Chugh 
> Signed-off-by: Stefan Hajnoczi 
> ---

> +3. Assign groups to the test
> +
> +Add your test to the ./group file.  This file is the index of tests and 
> assigns
> +them to functional groups like "rw" for read-write tests.  Most tests belong 
> to
> +the "rw" and "auto" groups.  "auto" means the test runs when ./check is 
> invoked
> +without a -g argument.
> +
> +Consider adding your test to the "quick" group if it executes quickly (<1s).

We have several tests going up to 5s (and I have a patch pending to
remove two tests that took longer) - I think 1s is a bit on the short
end for still classifying a test as quick.

> +This group is run by "make check-block" and is often included as part of 
> build
> +tests in continuous integration systems.

It would still be nice to have 'make check' run 'make check-block'...
but that's independent of this patch.

> +Once you are happy with the test output it can be used as the golden master
> +with "mv .out.bad .out".  Rerun the test to verify
> +that it passes.
> +
> +Congratulations, you've created a new test!

Maybe a reminder to 'git add' the new files, then submit the patch?

Better than what we have, so whether or not you make further tweaks
according to my suggestions,
Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature