docs: Add docs/devel/testing.rst

To make our efforts on QEMU testing easier to consume by contributors,
let's add a document. For example, Patchew reports build errors on
patches that should be relativly easy to reproduce with a few steps, and
it is much nicer if there is such a documentation that it can refer to.

This focues on how to run existing tests and how to write new test
cases, without going into the frameworks themselves.

Signed-off-by: Fam Zheng <famz@redhat.com>

---

To reviewers: though we don't have dedicate maintainers on tests or
docs, there are a few sections that are closely related to maintained
areas, such as block layer, QAPI, docker/vm based testings and image
fuzzer (would fuzzier be more correct?). It is perfect if you review
only the part you maintain or are interested in.
---
 docs/devel/testing.rst | 449 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 449 insertions(+)
 create mode 100644 docs/devel/testing.rst

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180129033133.31288-1-famz@redhat.com
Subject: [Qemu-devel] [PATCH] docs: Add docs/devel/testing.rst

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]            patchew/151700552398.7196.2573848773899364520.stgit@bahia.lan -> patchew/151700552398.7196.2573848773899364520.stgit@bahia.lan
 * [new tag]               patchew/20180129033133.31288-1-famz@redhat.com -> patchew/20180129033133.31288-1-famz@redhat.com
Switched to a new branch 'test'
a431f9c793 docs: Add docs/devel/testing.rst

=== OUTPUT BEGIN ===
Checking PATCH 1/1: docs: Add docs/devel/testing.rst...
ERROR: trailing whitespace
#190: FILE: docs/devel/testing.rst:167:
+  ./check -nbd $

total: 1 errors, 0 warnings, 449 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

On Mon, Jan 29, 2018 at 11:31 AM, Fam Zheng <famz@redhat.com> wrote:
> +VM testing
> +==========

I should have noted that this section is not new but an md->rst
convert of ./tests/vm/README. The latter should be made a pointer to
this one once it is merged.

Fam

On 29.01.2018 04:31, Fam Zheng wrote:
[...]
> +1. Create a new source file. For example, ``tests/test-foo.c``.
[...]
> +1. Create a new source file for the test. (More than one file can be added as
> +   necessary.) For example, ``tests/test-foo-device.c``.  2. Write the test
> +   code with the glib and libqtest/libqos API. See also existing tests and the
> +   library headers for reference.

Can you please name the examples tests/foo-test.c and
tests/foo-device-test.c instead? Then the binaries will be covered by
the wildcard in tests/.gitignore automatically. Otherwise the list in
that gitignore file will grow and grow... Maybe it's also worth to
mention the naming scheme in this doc file?

> +Image fuzzier testing
> +=====================
> +
> +An image fuzzier was added to exercise format drivers. Currently only qcow2 is
> +supported. To start the fuzzier, run

s/fuzzier/fuzzer/g ?

 Thomas

On Mon, Jan 29, 2018 at 2:56 PM, Thomas Huth <thuth@redhat.com> wrote:
> On 29.01.2018 04:31, Fam Zheng wrote:
> [...]
>> +1. Create a new source file. For example, ``tests/test-foo.c``.
> [...]
>> +1. Create a new source file for the test. (More than one file can be added as
>> +   necessary.) For example, ``tests/test-foo-device.c``.  2. Write the test
>> +   code with the glib and libqtest/libqos API. See also existing tests and the
>> +   library headers for reference.
>
> Can you please name the examples tests/foo-test.c and
> tests/foo-device-test.c instead? Then the binaries will be covered by
> the wildcard in tests/.gitignore automatically. Otherwise the list in
> that gitignore file will grow and grow... Maybe it's also worth to
> mention the naming scheme in this doc file?

Yeah, I can do it. It's just I fail to find a definitive guideline on
test-* vs *-test for the names, and the numbers are neck-and-neck:

fam@lemon:~/work/qemu [master]$ ls tests/*test.c | wc -l
67
fam@lemon:~/work/qemu [master]$ ls tests/test-*.c | wc -l
68

>
>> +Image fuzzier testing
>> +=====================
>> +
>> +An image fuzzier was added to exercise format drivers. Currently only qcow2 is
>> +supported. To start the fuzzier, run
>
> s/fuzzier/fuzzer/g ?

Okay.

Thanks for reviewing!

Fam

Fam Zheng <famz@redhat.com> writes:

> To make our efforts on QEMU testing easier to consume by contributors,
> let's add a document. For example, Patchew reports build errors on
> patches that should be relativly easy to reproduce with a few steps, and
> it is much nicer if there is such a documentation that it can refer to.
>
> This focues on how to run existing tests and how to write new test
> cases, without going into the frameworks themselves.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
>
> ---
>
> To reviewers: though we don't have dedicate maintainers on tests or
> docs, there are a few sections that are closely related to maintained
> areas, such as block layer, QAPI, docker/vm based testings and image
> fuzzer (would fuzzier be more correct?). It is perfect if you review
> only the part you maintain or are interested in.
> ---
>  docs/devel/testing.rst | 449 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 449 insertions(+)
>  create mode 100644 docs/devel/testing.rst
>
> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
> new file mode 100644
> index 0000000000..16217e4fa2
> --- /dev/null
> +++ b/docs/devel/testing.rst
> @@ -0,0 +1,449 @@
> +===============
> +Testing in QEMU
> +===============
> +
> +This document describes the testing infrastructure in QEMU.
> +
> +"Make check" testings
> +=====================
> +
> +The "make check" testing family includes most of the C based tests in QEMU. For
> +a quick help, run ``make check-help`` from the source tree.
> +
> +The usual way to run these tests is:
> +
> +.. code::
> +
> +  make check
> +
> +which includes QAPI schema tests, unit tests, and QTests. Different sub-types
> +of "make check" testings will be explained below.
> +
> +Before running tests, it is best to build QEMU programs first. Some tests
> +expect the executables to exist and will fail with obscure messages if cannot
> +find them.
> +
> +Unit tests
> +----------
> +
> +Unit tests, which can be invoked with ``make check-unit``, are simple C tests
> +that typically link to individual QEMU objects and exercise them by calling
> +into the modules.
> +
> +If you are writing new code in QEMU, consider adding a unit test, especially
> +for utility modules that are relatively stateless or have few dependencies. To
> +add a new unit test:
> +
> +1. Create a new source file. For example, ``tests/test-foo.c``.
> +
> +2. Write the test. Normally you would include the headers file which exports
> +   the module API, then verify the interface behaves as expected from your
> +   test. The test code should be organized with the glib testing framework.
> +   Copy and modify an existing test is usually a good idea.
> +
> +3. Add the test to ``tests/Makefile.include``. First, name the unit test
> +   program and add it to ``$(check-unit-y)``; then add a rule to build the
> +   executable. Optionally, you can add a magical variable to support ``gcov``.
> +   For example:
> +
> +.. code::
> +
> +  check-unit-y += tests/test-foo$(EXESUF)
> +  tests/test-foo$(EXESUF): tests/test-foo.o $(test-util-obj-y)
> +  ...
> +  gcov-files-test-foo-y = util/foo.c
> +
> +QTest
> +-----
> +
> +QTest is a testing framework that simplifies starting QEMU and interacting with
> +the virtual machine just like a guest kernel does. It can be very useful to
> +test hardware emulation, for example; it could also control certain aspects of
> +QEMU (such as virtual clock stepping), with a specially purposed "qtest"
> +protocol. Refer to the documentation in ``qtest.c`` file for more details of
> +the protocol.
> +
> +QTest cases can be executed with
> +
> +.. code::
> +
> +   make check-qtest
> +
> +The QTest library is implemented by ``tests/libqtest.c`` and the API is defined
> +in ``tests/libqtest.h``.
> +
> +Consider adding a new QTest case when you are introducing a new virtual
> +hardware, or extending one if you are adding functionalities to an existing
> +virtual device.
> +
> +On top of libqtest, a higher level library, ``libqos``, was created to
> +encapsulate common tasks of device drivers, such as memory management and
> +communicating with system buses or devices. Many virtual device tests use
> +libqos instead of directly calling into libqos.
> +
> +Steps to add a new QTest case are:
> +
> +1. Create a new source file for the test. (More than one file can be added as
> +   necessary.) For example, ``tests/test-foo-device.c``.  2. Write the test
> +   code with the glib and libqtest/libqos API. See also existing tests and the
> +   library headers for reference.
> +
> +3. Register the new test in ``tests/Makefile.include``. Add the test executable
> +   name to an appropriate ``check-qtest-*-y`` variable. For example:
> +
> +   ``check-qtest-generic-y = tests/test-foo-device$(EXESUF)``
> +
> +4. Add object dependencies of the executable in the Makefile, including the
> +   test source file(s) and other interesting objects. For example:
> +
> +   ``tests/test-foo-device$(EXESUF): tests/test-foo-device.o $(libqos-obj-y)``
> +
> +QAPI schema tests
> +-----------------
> +
> +The QAPI schema tests validate the QAPI parser used by QMP, by feeding
> +predefined input to the parser and compare the result with the reference
> +output.
> +
> +The input/output data is managed under the ``tests/qapi-schema`` directory.
> +Each test case includes four files that have a common base name:
> +
> +  * ``${casename}.json`` - the file contains the JSON input for feeding the
> +    parser
> +  * ``${casename}.out`` - the file contains the expected stdout from the parser
> +  * ``${casename}.err`` - the file contains the expected stderr from the parser
> +  * ``${casename}.exit`` - the expected error code
> +
> +Consider adding a new QAPI schema test when you are making a change on the QAPI
> +parser (either fixing a bug or extending/modifying the syntax). To do this:
> +
> +1. Add four files for the new case as explained above. For example:
> +
> +  ``$EDITOR tests/qapi-schema/foo.{json,out,err,exit}``.
> +
> +2. Add the new test in ``tests/Makefile.include``. For example:
> +
> +  ``qapi-schema += foo.json``
> +
> +check-block
> +-----------
> +
> +``make check-block`` is a legacy command to invoke block layer iotests and is
> +rarely used. See "QEMU iotests" section below for more information.
> +
> +GCC gcov support
> +----------------
> +
> +``gcov`` is a GCC tool to analyze the testing coverage by instrumenting the
> +tested code. To use it, configure QEMU with ``--enable-gcov`` option and build.
> +Then run ``make check`` as usual. There will be additional ``gcov`` output as
> +the testing goes on, showing the test coverage percentage numbers per analyzed
> +source file. More detailed reports can be obtained by running ``gcov`` command
> +on the output files under ``$build_dir/tests/``, please read the ``gcov``
> +documentation for more information.
> +
> +QEMU iotests
> +============
> +
> +QEMU iotests, under the directory ``tests/qemu-iotests``, is the testing
> +framework widely used to test block layer related features. It is higher level
> +than "make check" tests and 99% of the code is written in bash or Python
> +scripts.  The testing success criteria is golden output comparison, and the
> +test files are named with numbers.
> +
> +To run iotests, make sure QEMU is built successfully, then switch to the
> +``tests/qemu-iotests`` directory under the build directory, and run ``./check``
> +with desired arguments from there.
> +
> +By default, "raw" format and "file" protocol is used; all tests will be
> +executed, except the unsupported ones. You can override the format and protocol
> +with arguments:
> +
> +.. code::
> +
> +  # test with qcow2 format
> +  ./check -qcow2
> +  # or test a different protocol
> +  ./check -nbd
> +
> +It's also possible to list test numbers explicitly:
> +
> +.. code::
> +
> +  # run selected cases with qcow2 format
> +  ./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.
> +
> +More options are supported by the ``./check`` script, run ``./check -h`` for
> +help.
> +
> +Writing a new test case
> +-----------------------
> +
> +Consider writing a tests case when you are making any changes to the block
> +layer. An iotest case is usually the choice for that. There are already many
> +test cases, so it is possible that extending one of them may achieve the goal
> +and save the boilerplate to create one.  (Unfortunately, there isn't a 100%
> +reliable way to find a related one out of hundreds of tests.  One approach is
> +using ``git grep``.)
> +
> +Usually an iotest case consists of two files. One is an executable that
> +produces output to stdout and stderr, the other is the expected reference
> +output. They are given the same number in file names. E.g. Test script ``055``
> +and reference output ``055.out``.
> +
> +In rare cases, when outputs differ between cache mode ``none`` and others, a
> +``.out.nocache`` file is added. In other cases, when outputs differ between
> +image formats, more than one ``.out`` files are created ending with the
> +respective format names, e.g. ``178.out.qcow2`` and ``178.out.raw``.
> +
> +There isn't a hard rule about how to write a test script, but a new test is
> +usually a (copy and) modification of an existing case.  There are a few
> +commonly used ways to create a test:
> +
> +* A Bash script. It will make use of several environmental variables related
> +  to the testing procedure, and could source a group of ``common.*`` libraries
> +  for some common helper routines.
> +
> +* A Python unittest script. Import ``iotests`` and create a subclass of
> +  ``iotests.QMPTestCase``, then call ``iotests.main`` method. The downside of
> +  this approach is that the output is too scarce, and the script is considered
> +  harder to debug.
> +
> +* A simple Python script without using unittest module. This could also import
> +  ``iotests`` for launching QEMU and utilities etc, but it doesn't inherit
> +  from ``iotests.QMPTestCase`` therefore doesn't use the Python unittest
> +  execution. This is a combination of 1 and 2.
> +
> +Pick the language per your preference since both Bash and Python have
> +comparable library support for invoking and interacting with QEMU programs. If
> +you opt for Python, it is strongly recommended to write Python 3 compatible
> +code.
> +
> +Docker based testing
> +====================
> +
> +Introduction
> +------------
> +
> +Docker testing framework in QEMU utilizes the public Docker images to build and
> +test QEMU in predefined and widely accessible Linux environments.  This makes
> +it possible to expand the test coverage across distros, toolchain flavors and
> +library versions.
> +
> +Prerequisites
> +-------------
> +
> +Install "docker" with the system package manager and start the Docker service
> +on your development machine, then make sure you have the privilege to run
> +Docker commands. Typically it means setting up passwordless ``sudo docker``
> +command or login as root.

The third option (which I run) is using group permissions so you can add
users to the docker group and make the socket group accessible via the
daemon.

It should be pointed out that both the sudo and group methods do open up
your machine to being exploited via docker's ability to bind mount
stuff. That is to say it's fine for developers on their own machine but
you might not want to do that on shared servers.

> For example:
> +
> +.. code::
> +
> +  $ sudo yum install docker
> +  $ # or `apt-get install docker` for Ubuntu, etc.
> +  $ sudo systemctl start docker
> +  $ sudo docker ps
> +
> +The last command should print an empty table, to verify the system is ready.
> +
> +Quickstart
> +----------
> +
> +From source tree, type ``make docker`` to see the help. A typical Docker
> +testing can be started without configuring or building QEMU (``configure`` and
> +``make`` are done in the container, with parameters defined by the make
> +target):
> +
> +.. code::
> +
> +  make docker-test-build@min-glib
> +
> +This will create a container instance using the ``min-glib`` image (the image
> +is downloaded and initialized automatically), in which the ``test-build`` job
> +is executed.
> +
> +Images
> +------
> +
> +Along with many other images, the ``min-glib`` image is defined in a Dockerfile
> +in ``tests/docker/dockefiles/``, called ``min-glib.docker``. ``make docker``
> +command will list all the available images.
> +
> +To add a new image, simply create a new ``.docker`` file under the
> +``tests/docker/dockerfiles/`` directory.
> +
> +A ``.pre`` script can be added beside the ``.docker`` file, which will be
> +executed before building the image under the build context directory. This is
> +mainly used to do necessary host side setup. One such setup is ``binfmt_misc``,
> +for example, to make qemu-user powered cross build containers work.
> +
> +Tests
> +-----
> +
> +Different tests are added to cover various configurations to build and test
> +QEMU.  Docker tests are the executables under ``tests/docker`` named
> +``test-*``. They are typically shell scripts and are built on top of a shell
> +library, ``tests/docker/common.rc``, which provides helpers to find the QEMU
> +source and build it.
> +
> +The full list of tests is printed in the ``make docker`` help.
> +
> +Tools
> +-----
> +
> +There are executables that are created to run in a specific Docker environment.
> +This makes it easy to write scripts that have heavy or special dependencies,
> +but are still very easy to use.
> +
> +Currently the only tool is ``travis``, which mimics the Travis-CI testings in a
> +container. It runs in the ``travis`` image:
> +
> +.. code::
> +
> +  make docker-travis@travis
> +
> +Debugging a Docker test failure
> +-------------------------------
> +
> +When CI tasks, maintainers or yourself report a Docker test failure, follow the
> +below steps to debug it:
> +
> +1. Locally reproduce the failure with the reported command line. E.g. run
> +   ``make docker-test-mingw@fedora J=8``.
> +2. Add "V=1" to the command line, try again, to see the verbose output.
> +3. Further add "DEBUG=1" to the command line. This will pause in a shell prompt
> +   in the container right before testing starts. You could either manually
> +   build QEMU and run tests from there, or press Ctrl-D to let the Docker
> +   testing continue.
> +4. If you press Ctrl-D, the same building and testing procedure will begin, and
> +   will hopefully run into the error again. After that, you will be dropped to
> +   the prompt for debug.
> +
> +Options
> +-------
> +
> +Various options can be used to affect how Docker testings are done. The full
> +list is in the ``make docker`` help text. The frequently used ones are:
> +
> +* ``V=1``: the same as in top level ``make``. It will be propagated to the
> +  container and enable verbose output.
> +* ``J=$N``: the number of parallel tasks in make commands in the container,
> +  similar to the ``-j $N`` option in top level ``make``. (The ``-j`` option in
> +  top level ``make`` will not be propagated into the container.)
> +* ``DEBUG=1``: enables debug. See the previous "Debugging a Docker test
> +  failure" section.
> +
> +VM testing
> +==========
> +
> +This test suite contains scripts that bootstrap various guest images that have
> +necessary packages to build QEMU. The basic usage is documented in ``Makefile``
> +help which is displayed with ``make vm-test``.
> +
> +Quick start
> +-----------
> +
> +Run ``make vm-test`` to list available make targets. Invoke a specific make
> +command to run build test in an image. For example, ``make vm-build-freebsd``
> +will build the source tree in the FreeBSD image. The command can be executed
> +from either the source tree or the build dir; if the former, ``./configure`` is
> +not needed. The command will then generate the test image in ``./tests/vm/``
> +under the working directory.
> +
> +Note: images created by the scripts accept a well-known RSA key pair for SSH
> +access, so they SHOULD NOT be exposed to external interfaces if you are
> +concerned about attackers taking control of the guest and potentially
> +exploiting a QEMU security bug to compromise the host.
> +
> +QEMU binary
> +-----------
> +
> +By default, qemu-system-x86_64 is searched in $PATH to run the guest. If there
> +isn't one, or if it is older than 2.10, the test won't work. In this case,
> +provide the QEMU binary in env var: ``QEMU=/path/to/qemu-2.10+``.
> +
> +Make jobs
> +---------
> +
> +The ``-j$X`` option in the make command line is not propagated into the VM,
> +specify ``J=$X`` to control the make jobs in the guest.
> +
> +Debugging
> +---------
> +
> +Add ``DEBUG=1`` and/or ``V=1`` to the make command to allow interactive
> +debugging and verbose output. If this is not enough, see the next section.
> +
> +Manual invocation
> +-----------------
> +
> +Each guest script is an executable script with the same command line options.
> +For example to work with the netbsd guest, use ``$QEMU_SRC/tests/vm/netbsd``:
> +
> +.. code::
> +
> +    $ cd $QEMU_SRC/tests/vm
> +
> +    # To bootstrap the image
> +    $ ./netbsd --build-image --image /var/tmp/netbsd.img
> +    <...>
> +
> +    # To run an arbitrary command in guest (the output will not be echoed unless
> +    # --debug is added)
> +    $ ./netbsd --debug --image /var/tmp/netbsd.img uname -a
> +
> +    # To build QEMU in guest
> +    $ ./netbsd --debug --image /var/tmp/netbsd.img --build-qemu $QEMU_SRC
> +
> +    # To get to an interactive shell
> +    $ ./netbsd --interactive --image /var/tmp/netbsd.img sh
> +
> +Adding new guests
> +-----------------
> +
> +Please look at existing guest scripts for how to add new guests.
> +
> +Most importantly, create a subclass of BaseVM and implement ``build_image()``
> +method and define ``BUILD_SCRIPT``, then finally call ``basevm.main()`` from
> +the script's ``main()``.
> +
> +* Usually in ``build_image()``, a template image is downloaded from a
> +  predefined URL. ``BaseVM._download_with_cache()`` takes care of the cache and
> +  the checksum, so consider using it.
> +
> +* Once the image is downloaded, users, SSH server and QEMU build deps should
> +  be set up:
> +
> +  - Root password set to ``BaseVM.ROOT_PASS``
> +  - User ``BaseVM.GUEST_USER`` is created, and password set to
> +    ``BaseVM.GUEST_PASS``
> +  - SSH service is enabled and started on boot,
> +    ``$QEMU_SRC/tests/keys/id_rsa.pub`` is added to ssh's ``authorized_keys``
> +    file of both root and the normal user
> +  - DHCP client service is enabled and started on boot, so that it can
> +    automatically configure the virtio-net-pci NIC and communicate with QEMU
> +    user net (10.0.2.2)
> +  - Necessary packages are installed to untar the source tarball and build
> +    QEMU
> +
> +* Write a proper ``BUILD_SCRIPT`` template, which should be a shell script that
> +  untars a raw virtio-blk block device, which is the tarball data blob of the
> +  QEMU source tree, then configure/build it. Running "make check" is also
> +  recommended.
> +
> +Image fuzzier testing
> +=====================
> +
> +An image fuzzier was added to exercise format drivers. Currently only qcow2 is
> +supported. To start the fuzzier, run
> +
> +.. code::
> +
> +  tests/image-fuzzer/runner.py -c '[["qemu-img", "info", "$test_img"]]' /tmp/test qcow2
> +
> +Alternatively, some command different from "qemu-img info" can be tested, by
> +changing the ``-c`` option.


--
Alex Bennée

On Mon, Jan 29, 2018 at 11:31:33AM +0800, Fam Zheng wrote:

Thanks for writing this!

I have only reviewed some parts in detail.

> +"Make check" testings
> +=====================
> +
> +The "make check" testing family includes most of the C based tests in QEMU. For
> +a quick help, run ``make check-help`` from the source tree.
> +
> +The usual way to run these tests is:
> +
> +.. code::
> +
> +  make check
> +
> +which includes QAPI schema tests, unit tests, and QTests. Different sub-types
> +of "make check" testings will be explained below.
> +
> +Before running tests, it is best to build QEMU programs first. Some tests
> +expect the executables to exist and will fail with obscure messages if cannot

s/if cannot/if they cannot/

> +find them.
> +
> +Unit tests
> +----------
> +
> +Unit tests, which can be invoked with ``make check-unit``, are simple C tests
> +that typically link to individual QEMU objects and exercise them by calling

I think this is clearer:
s/objects/object files/

Otherwise people might think "QEMU objects" is releated to QOM objects.

> +into the modules.

The term "modules" hasn't been defined.  I think the following is
clearer:

s/into the modules/exported functions/

> +
> +If you are writing new code in QEMU, consider adding a unit test, especially
> +for utility modules that are relatively stateless or have few dependencies. To
> +add a new unit test:
> +
> +1. Create a new source file. For example, ``tests/test-foo.c``.
> +
> +2. Write the test. Normally you would include the headers file which exports

s/headers/header/

> +   the module API, then verify the interface behaves as expected from your
> +   test. The test code should be organized with the glib testing framework.
> +   Copy and modify an existing test is usually a good idea.

s/Copy and modify/Copying and modifying/

> +
> +3. Add the test to ``tests/Makefile.include``. First, name the unit test
> +   program and add it to ``$(check-unit-y)``; then add a rule to build the
> +   executable. Optionally, you can add a magical variable to support ``gcov``.
> +   For example:
> +
> +.. code::
> +
> +  check-unit-y += tests/test-foo$(EXESUF)
> +  tests/test-foo$(EXESUF): tests/test-foo.o $(test-util-obj-y)
> +  ...
> +  gcov-files-test-foo-y = util/foo.c
> +
> +QTest
> +-----
> +
> +QTest is a testing framework that simplifies starting QEMU and interacting with

"simplifies starting QEMU" is not the most important aspect of qtest and
I would drop this completely.

The following single-sentence summary is a good opener: QTest is a
device emulation testing framework.

> +the virtual machine just like a guest kernel does. It can be very useful to
> +test hardware emulation, for example; it could also control certain aspects of
> +QEMU (such as virtual clock stepping), with a specially purposed "qtest"

s/a specially purposed/the special purpose/

> +protocol. Refer to the documentation in ``qtest.c`` file for more details of

s/in ``qtest.c`` file/in ``qtest.c``/

(If you want to keep "file" then a definite article is needed - "the".)

> +the protocol.
> +
> +QTest cases can be executed with
> +
> +.. code::
> +
> +   make check-qtest
> +
> +The QTest library is implemented by ``tests/libqtest.c`` and the API is defined
> +in ``tests/libqtest.h``.
> +
> +Consider adding a new QTest case when you are introducing a new virtual
> +hardware, or extending one if you are adding functionalities to an existing
> +virtual device.
> +
> +On top of libqtest, a higher level library, ``libqos``, was created to
> +encapsulate common tasks of device drivers, such as memory management and
> +communicating with system buses or devices. Many virtual device tests use

s/buses/busses/

> +libqos instead of directly calling into libqos.
> +
> +Steps to add a new QTest case are:
> +
> +1. Create a new source file for the test. (More than one file can be added as
> +   necessary.) For example, ``tests/test-foo-device.c``.  2. Write the test
> +   code with the glib and libqtest/libqos API. See also existing tests and the
> +   library headers for reference.
> +
> +3. Register the new test in ``tests/Makefile.include``. Add the test executable
> +   name to an appropriate ``check-qtest-*-y`` variable. For example:
> +
> +   ``check-qtest-generic-y = tests/test-foo-device$(EXESUF)``
> +
> +4. Add object dependencies of the executable in the Makefile, including the
> +   test source file(s) and other interesting objects. For example:
> +
> +   ``tests/test-foo-device$(EXESUF): tests/test-foo-device.o $(libqos-obj-y)``
> +
> +QAPI schema tests
> +-----------------
> +
> +The QAPI schema tests validate the QAPI parser used by QMP, by feeding
> +predefined input to the parser and compare the result with the reference

s/compare/comparing/

On 01/29/2018 01:42 AM, Fam Zheng wrote:
> On Mon, Jan 29, 2018 at 2:56 PM, Thomas Huth <thuth@redhat.com> wrote:
>> On 29.01.2018 04:31, Fam Zheng wrote:
>> [...]
>>> +1. Create a new source file. For example, ``tests/test-foo.c``.
>> [...]
>>> +1. Create a new source file for the test. (More than one file can be added as
>>> +   necessary.) For example, ``tests/test-foo-device.c``.  2. Write the test
>>> +   code with the glib and libqtest/libqos API. See also existing tests and the
>>> +   library headers for reference.
>>
>> Can you please name the examples tests/foo-test.c and
>> tests/foo-device-test.c instead? Then the binaries will be covered by
>> the wildcard in tests/.gitignore automatically. Otherwise the list in
>> that gitignore file will grow and grow... Maybe it's also worth to
>> mention the naming scheme in this doc file?
> 
> Yeah, I can do it. It's just I fail to find a definitive guideline on
> test-* vs *-test for the names, and the numbers are neck-and-neck:
> 
> fam@lemon:~/work/qemu [master]$ ls tests/*test.c | wc -l
> 67
> fam@lemon:~/work/qemu [master]$ ls tests/test-*.c | wc -l
> 68

I'm all for a mass rename to the suffix style (as it is easier to write
a gitignore suffix), if no one can provide a good reason for the naming
difference.  (At one point, the difference was based on whether the test
was standalone or run as part of 'make check-qtest' - but I don't think
even that rule applies very well to the current state).

On 01/29/2018 05:04 AM, Stefan Hajnoczi wrote:
> On Mon, Jan 29, 2018 at 11:31:33AM +0800, Fam Zheng wrote:
> 
> Thanks for writing this!
> 
> I have only reviewed some parts in detail.
> 

>> +
>> +On top of libqtest, a higher level library, ``libqos``, was created to
>> +encapsulate common tasks of device drivers, such as memory management and
>> +communicating with system buses or devices. Many virtual device tests use
> 
> s/buses/busses/

That's one of those stupid English words where both spellings are valid,
and which one you get is a matter of preference (I can't even call it a
US-vs.-UK issue).

On 01/28/2018 09:31 PM, Fam Zheng wrote:
> To make our efforts on QEMU testing easier to consume by contributors,
> let's add a document. For example, Patchew reports build errors on
> patches that should be relativly easy to reproduce with a few steps, and

s/relativly/relatively/

> it is much nicer if there is such a documentation that it can refer to.
> 
> This focues on how to run existing tests and how to write new test

s/focues/focuses/

> cases, without going into the frameworks themselves.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> 
> ---
> 

I'll try not to repeat comments made by others...

> +This document describes the testing infrastructure in QEMU.
> +
> +"Make check" testings
> +=====================

s/testings/tests/, or 'Testing with "make check"'

> +
> +The "make check" testing family includes most of the C based tests in QEMU. For
> +a quick help, run ``make check-help`` from the source tree.
> +
> +The usual way to run these tests is:
> +
> +.. code::
> +
> +  make check
> +
> +which includes QAPI schema tests, unit tests, and QTests. Different sub-types
> +of "make check" testings will be explained below.

s/testings/tests/

> +
> +Before running tests, it is best to build QEMU programs first. Some tests
> +expect the executables to exist and will fail with obscure messages if cannot
> +find them.

Should we fix 'make check' to depend on 'make all', so that we don't
have to require this?

> +
> +Unit tests
> +----------
> +
> +Unit tests, which can be invoked with ``make check-unit``, are simple C tests
> +that typically link to individual QEMU objects and exercise them by calling
> +into the modules.
> +
> +If you are writing new code in QEMU, consider adding a unit test, especially
> +for utility modules that are relatively stateless or have few dependencies. To
> +add a new unit test:
> +
> +1. Create a new source file. For example, ``tests/test-foo.c``.
> +
> +2. Write the test. Normally you would include the headers file which exports
> +   the module API, then verify the interface behaves as expected from your
> +   test. The test code should be organized with the glib testing framework.
> +   Copy and modify an existing test is usually a good idea.
> +
> +3. Add the test to ``tests/Makefile.include``. First, name the unit test
> +   program and add it to ``$(check-unit-y)``; then add a rule to build the
> +   executable. Optionally, you can add a magical variable to support ``gcov``.
> +   For example:
> +
> +.. code::
> +
> +  check-unit-y += tests/test-foo$(EXESUF)
> +  tests/test-foo$(EXESUF): tests/test-foo.o $(test-util-obj-y)
> +  ...
> +  gcov-files-test-foo-y = util/foo.c

Is it worth documenting that you can often run 'gdb ./test-foo' after
the fact for a failing test?  (Most unit tests don't require any magic
environment variables to be set)

> +
> +QTest
> +-----
> +
> +QTest is a testing framework that simplifies starting QEMU and interacting with
> +the virtual machine just like a guest kernel does. It can be very useful to
> +test hardware emulation, for example; it could also control certain aspects of
> +QEMU (such as virtual clock stepping), with a specially purposed "qtest"
> +protocol. Refer to the documentation in ``qtest.c`` file for more details of
> +the protocol.
> +
> +QTest cases can be executed with
> +
> +.. code::
> +
> +   make check-qtest
> +
> +The QTest library is implemented by ``tests/libqtest.c`` and the API is defined
> +in ``tests/libqtest.h``.
> +
> +Consider adding a new QTest case when you are introducing a new virtual
> +hardware, or extending one if you are adding functionalities to an existing
> +virtual device.
> +
> +On top of libqtest, a higher level library, ``libqos``, was created to
> +encapsulate common tasks of device drivers, such as memory management and
> +communicating with system buses or devices. Many virtual device tests use
> +libqos instead of directly calling into libqos.
> +
> +Steps to add a new QTest case are:
> +
> +1. Create a new source file for the test. (More than one file can be added as
> +   necessary.) For example, ``tests/test-foo-device.c``.  2. Write the test
> +   code with the glib and libqtest/libqos API. See also existing tests and the
> +   library headers for reference.
> +
> +3. Register the new test in ``tests/Makefile.include``. Add the test executable
> +   name to an appropriate ``check-qtest-*-y`` variable. For example:
> +
> +   ``check-qtest-generic-y = tests/test-foo-device$(EXESUF)``
> +
> +4. Add object dependencies of the executable in the Makefile, including the
> +   test source file(s) and other interesting objects. For example:
> +
> +   ``tests/test-foo-device$(EXESUF): tests/test-foo-device.o $(libqos-obj-y)``

Is it worth documenting that 'make check-qtest' runs EVERY test unless
you define magic environment variables; and if it fails, running 'make
check-qtest V=1' can help identify which test failed?  Here, running a
mere 'gdb foo-device-test' is prone to fail (because it is missing magic
environment variables); it's possible to run individual tests under gdb
but it requires more effort (among other things, the V=1 helps you learn
what environment variables to set).

> +

> +Docker based testing
> +====================

'Docker based tests' or 'Testing with docker'

> +
> +Introduction
> +------------
> +
> +Docker testing framework in QEMU utilizes the public Docker images to build and

s/^Docker/The Docker/
s/the public/public/

> +test QEMU in predefined and widely accessible Linux environments.  This makes
> +it possible to expand the test coverage across distros, toolchain flavors and
> +library versions.
> +

On 29.01.2018 15:36, Eric Blake wrote:
> On 01/29/2018 01:42 AM, Fam Zheng wrote:
>> On Mon, Jan 29, 2018 at 2:56 PM, Thomas Huth <thuth@redhat.com> wrote:
>>> On 29.01.2018 04:31, Fam Zheng wrote:
>>> [...]
>>>> +1. Create a new source file. For example, ``tests/test-foo.c``.
>>> [...]
>>>> +1. Create a new source file for the test. (More than one file can be added as
>>>> +   necessary.) For example, ``tests/test-foo-device.c``.  2. Write the test
>>>> +   code with the glib and libqtest/libqos API. See also existing tests and the
>>>> +   library headers for reference.
>>>
>>> Can you please name the examples tests/foo-test.c and
>>> tests/foo-device-test.c instead? Then the binaries will be covered by
>>> the wildcard in tests/.gitignore automatically. Otherwise the list in
>>> that gitignore file will grow and grow... Maybe it's also worth to
>>> mention the naming scheme in this doc file?
>>
>> Yeah, I can do it. It's just I fail to find a definitive guideline on
>> test-* vs *-test for the names, and the numbers are neck-and-neck:
>>
>> fam@lemon:~/work/qemu [master]$ ls tests/*test.c | wc -l
>> 67
>> fam@lemon:~/work/qemu [master]$ ls tests/test-*.c | wc -l
>> 68
> 
> I'm all for a mass rename to the suffix style (as it is easier to write
> a gitignore suffix), if no one can provide a good reason for the naming
> difference. 

Good idea. And while we're at it, I think we should also sort the tests
into subfolders, e.g. tests/qtest/ and tests/unit/ etc.

 Thomas

On Mon, Jan 29, 2018 at 08:38:29AM -0600, Eric Blake wrote:
> On 01/29/2018 05:04 AM, Stefan Hajnoczi wrote:
> > On Mon, Jan 29, 2018 at 11:31:33AM +0800, Fam Zheng wrote:
> > 
> > Thanks for writing this!
> > 
> > I have only reviewed some parts in detail.
> > 
> 
> >> +
> >> +On top of libqtest, a higher level library, ``libqos``, was created to
> >> +encapsulate common tasks of device drivers, such as memory management and
> >> +communicating with system buses or devices. Many virtual device tests use
> > 
> > s/buses/busses/
> 
> That's one of those stupid English words where both spellings are valid,
> and which one you get is a matter of preference (I can't even call it a
> US-vs.-UK issue).

Thanks, I didn't know there were two spellings for this one.

Stefan

On 01/29/2018 10:10 AM, Stefan Hajnoczi wrote:
> On Mon, Jan 29, 2018 at 08:38:29AM -0600, Eric Blake wrote:
>> On 01/29/2018 05:04 AM, Stefan Hajnoczi wrote:
>>> On Mon, Jan 29, 2018 at 11:31:33AM +0800, Fam Zheng wrote:
>>>
>>> Thanks for writing this!
>>>
>>> I have only reviewed some parts in detail.
>>>
>>
>>>> +
>>>> +On top of libqtest, a higher level library, ``libqos``, was created to
>>>> +encapsulate common tasks of device drivers, such as memory management and
>>>> +communicating with system buses or devices. Many virtual device tests use
>>>
>>> s/buses/busses/
>>
>> That's one of those stupid English words where both spellings are valid,
>> and which one you get is a matter of preference (I can't even call it a
>> US-vs.-UK issue).
> 
> Thanks, I didn't know there were two spellings for this one.

https://www.merriam-webster.com/words-at-play/plural-of-bus

On Mon, Jan 29, 2018 at 10:51 PM, Eric Blake <eblake@redhat.com> wrote:
> On 01/28/2018 09:31 PM, Fam Zheng wrote:
>> To make our efforts on QEMU testing easier to consume by contributors,
>> let's add a document. For example, Patchew reports build errors on
>> patches that should be relativly easy to reproduce with a few steps, and
>
> s/relativly/relatively/
>
>> it is much nicer if there is such a documentation that it can refer to.
>>
>> This focues on how to run existing tests and how to write new test
>
> s/focues/focuses/
>
>> cases, without going into the frameworks themselves.
>>
>> Signed-off-by: Fam Zheng <famz@redhat.com>
>>
>> ---
>>
>
> I'll try not to repeat comments made by others...
>
>> +This document describes the testing infrastructure in QEMU.
>> +
>> +"Make check" testings
>> +=====================
>
> s/testings/tests/, or 'Testing with "make check"'

Sounds good, it will put "make" back to lower case again. :)

>
>> +
>> +The "make check" testing family includes most of the C based tests in QEMU. For
>> +a quick help, run ``make check-help`` from the source tree.
>> +
>> +The usual way to run these tests is:
>> +
>> +.. code::
>> +
>> +  make check
>> +
>> +which includes QAPI schema tests, unit tests, and QTests. Different sub-types
>> +of "make check" testings will be explained below.
>
> s/testings/tests/
>
>> +
>> +Before running tests, it is best to build QEMU programs first. Some tests
>> +expect the executables to exist and will fail with obscure messages if cannot
>> +find them.
>
> Should we fix 'make check' to depend on 'make all', so that we don't
> have to require this?

Maybe. We should try.

>
>> +
>> +Unit tests
>> +----------
>> +
>> +Unit tests, which can be invoked with ``make check-unit``, are simple C tests
>> +that typically link to individual QEMU objects and exercise them by calling
>> +into the modules.
>> +
>> +If you are writing new code in QEMU, consider adding a unit test, especially
>> +for utility modules that are relatively stateless or have few dependencies. To
>> +add a new unit test:
>> +
>> +1. Create a new source file. For example, ``tests/test-foo.c``.
>> +
>> +2. Write the test. Normally you would include the headers file which exports
>> +   the module API, then verify the interface behaves as expected from your
>> +   test. The test code should be organized with the glib testing framework.
>> +   Copy and modify an existing test is usually a good idea.
>> +
>> +3. Add the test to ``tests/Makefile.include``. First, name the unit test
>> +   program and add it to ``$(check-unit-y)``; then add a rule to build the
>> +   executable. Optionally, you can add a magical variable to support ``gcov``.
>> +   For example:
>> +
>> +.. code::
>> +
>> +  check-unit-y += tests/test-foo$(EXESUF)
>> +  tests/test-foo$(EXESUF): tests/test-foo.o $(test-util-obj-y)
>> +  ...
>> +  gcov-files-test-foo-y = util/foo.c
>
> Is it worth documenting that you can often run 'gdb ./test-foo' after
> the fact for a failing test?  (Most unit tests don't require any magic
> environment variables to be set)

Good point!

I'll also add a paragraph about MALLOC_PERTURB_ then.

>
>> +
>> +QTest
>> +-----
>> +
>> +QTest is a testing framework that simplifies starting QEMU and interacting with
>> +the virtual machine just like a guest kernel does. It can be very useful to
>> +test hardware emulation, for example; it could also control certain aspects of
>> +QEMU (such as virtual clock stepping), with a specially purposed "qtest"
>> +protocol. Refer to the documentation in ``qtest.c`` file for more details of
>> +the protocol.
>> +
>> +QTest cases can be executed with
>> +
>> +.. code::
>> +
>> +   make check-qtest
>> +
>> +The QTest library is implemented by ``tests/libqtest.c`` and the API is defined
>> +in ``tests/libqtest.h``.
>> +
>> +Consider adding a new QTest case when you are introducing a new virtual
>> +hardware, or extending one if you are adding functionalities to an existing
>> +virtual device.
>> +
>> +On top of libqtest, a higher level library, ``libqos``, was created to
>> +encapsulate common tasks of device drivers, such as memory management and
>> +communicating with system buses or devices. Many virtual device tests use
>> +libqos instead of directly calling into libqos.
>> +
>> +Steps to add a new QTest case are:
>> +
>> +1. Create a new source file for the test. (More than one file can be added as
>> +   necessary.) For example, ``tests/test-foo-device.c``.  2. Write the test
>> +   code with the glib and libqtest/libqos API. See also existing tests and the
>> +   library headers for reference.
>> +
>> +3. Register the new test in ``tests/Makefile.include``. Add the test executable
>> +   name to an appropriate ``check-qtest-*-y`` variable. For example:
>> +
>> +   ``check-qtest-generic-y = tests/test-foo-device$(EXESUF)``
>> +
>> +4. Add object dependencies of the executable in the Makefile, including the
>> +   test source file(s) and other interesting objects. For example:
>> +
>> +   ``tests/test-foo-device$(EXESUF): tests/test-foo-device.o $(libqos-obj-y)``
>
> Is it worth documenting that 'make check-qtest' runs EVERY test unless
> you define magic environment variables; and if it fails, running 'make
> check-qtest V=1' can help identify which test failed?  Here, running a
> mere 'gdb foo-device-test' is prone to fail (because it is missing magic
> environment variables); it's possible to run individual tests under gdb
> but it requires more effort (among other things, the V=1 helps you learn
> what environment variables to set).

Yes, will add a paragraph here too.

>
>> +
>
>> +Docker based testing
>> +====================
>
> 'Docker based tests' or 'Testing with docker'
>
>> +
>> +Introduction
>> +------------
>> +
>> +Docker testing framework in QEMU utilizes the public Docker images to build and
>
> s/^Docker/The Docker/
> s/the public/public/
>
>> +test QEMU in predefined and widely accessible Linux environments.  This makes
>> +it possible to expand the test coverage across distros, toolchain flavors and
>> +library versions.
>> +
>
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
>

On Mon, Jan 29, 2018 at 7:04 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Mon, Jan 29, 2018 at 11:31:33AM +0800, Fam Zheng wrote:
>
> Thanks for writing this!
>
> I have only reviewed some parts in detail.

Thanks for the review! I've made all the fixes (except for the buses
spelling. :).

Fam

On Mon, Jan 29, 2018 at 6:36 PM, Alex Bennée <alex.bennee@linaro.org> wrote:
> Fam Zheng <famz@redhat.com> writes:
>> +Prerequisites
>> +-------------
>> +
>> +Install "docker" with the system package manager and start the Docker service
>> +on your development machine, then make sure you have the privilege to run
>> +Docker commands. Typically it means setting up passwordless ``sudo docker``
>> +command or login as root.
>
> The third option (which I run) is using group permissions so you can add
> users to the docker group and make the socket group accessible via the
> daemon.
>
> It should be pointed out that both the sudo and group methods do open up
> your machine to being exploited via docker's ability to bind mount
> stuff. That is to say it's fine for developers on their own machine but
> you might not want to do that on shared servers.

Thanks, Alex, very good I've added that paragraph!

Fam