Re: [Qemu-devel] [PATCH V3 2/5] runner: Tool for fuzz tests execution

[Stefan Hajnoczi]

On Wed, Jul 16, 2014 at 11:49:36PM +0400, Maria Kustova wrote:
> +    def __init__(self, test_id, seed, work_dir, run_log,
> +                 cleanup=True, log_all=False):
> +        """Set test environment in a specified work directory.
> +
> +        Path to qemu-img and qemu-io will be retrieved from 'QEMU_IMG' and
> +        'QEMU_IO' environment variables
> +        """
> +        if seed is not None:
> +            self.seed = seed
> +        else:
> +            self.seed = str(hash(random.randint(0, sys.maxint)))

What is the purpose of hash()?  hash(<int>) == <int>.

> +    def finish(self):
> +        """ Restore environment after a test execution. Remove folders of
> +        passed tests
> +        """
> +        self.log.close()
> +        self.parent_log.close()
> +        os.chdir(self.init_path)
> +        if (False not in self.result) and self.cleanup:
> +            rmtree(self.current_dir)

self.result could simply be a bool:

self.failed = False
...
if self.cleanup and not self.failed:
    rmtree(self.current_dir)

The array seems unnecessary

> +
> +if __name__ == '__main__':
> +
> +    def usage():
> +        print """
> +        Usage: runner.py [OPTION...] TEST_DIR IMG_GENERATOR
> +
> +        Set up test environment in TEST_DIR and run a test in it. A module 
> for
> +        test image generation should be specified via IMG_GENERATOR.
> +        Example:
> +        runner.py -c '[["qemu-img", "info", "$test_img"]]' /tmp/test ../qcow2
> +
> +        Optional arguments:
> +          -h, --help                    display this help and exit
> +          -c, --command=JSON            run tests for all commands specified 
> in
> +                                        the JSON object
> +          -s, --seed=STRING             seed for a test image generation,
> +                                        by default will be generated randomly
> +          --config=JSON                 take fuzzer configuration from the 
> JSON
> +                                        object
> +          -k, --keep_passed             don't remove folders of passed tests
> +          -v, --verbose                 log information about passed tests
> +
> +        JSON objects:
> +
> +        '--command' accepts a JSON object containing a list of commands.
> +        Each command presents an application under test with all its 
> paramaters
> +        as a list of strings, e.g.
> +          ["qemu-io", "$test_img", "-c", "write $off $len"]

[] is not a JSON object.  {} is a JSON object.

Does this mean --command takes a JSON array of commands to invoke, where
each command is a JSON array of argument strings?

> +
> +        Supported application aliases: 'qemu-img' and 'qemu-io'.
> +        Supported argument aliases: $test_img for the fuzzed image, $off
> +        for an offset, $len for length.
> +
> +        Values for $off and $len will be generated based on the virtual disk
> +        size of the fuzzed image
> +        Paths to 'qemu-img' and 'qemu-io' are retrevied from 'QEMU_IMG' and
> +        'QEMU_IO' environment variables
> +
> +        '--config' accepts a JSON object containing a list of fields to be
> +        fuzzed, e.g.
> +          [["header"], ["header", "version"]]

Same thing about JSON "object" terminology.

> +    try:
> +        opts, args = getopt.gnu_getopt(sys.argv[1:], 'c:hs:kv',
> +                                       ['command=', 'help', 'seed=', 
> 'config=',
> +                                        'keep_passed', 'verbose'])
> +    except getopt.error:
> +        e = sys.exc_info()[1]

How about the more common except syntax:
except getopt.error, e:

It eliminates sys.exc_info()[1].  The same applies to the other
instances in this file.

> +    if not len(args) == 2:
> +        print "Missed parameter\nTry 'runner.py --help' " \

s/Missed parameter/Expected two parameters/

(More than 2 parameters could have been given.)

pgpC15b2Yn0ia.pgp
Description: PGP signature