All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alasdair G Kergon <agk@redhat.com>
To: lvm-devel@redhat.com
Subject: RFC: testing framework
Date: Mon, 17 Sep 2007 20:04:24 +0100	[thread overview]
Message-ID: <20070917190424.GL18444@agk.fab.redhat.com> (raw)
In-Reply-To: <87ejgxcxpn.fsf@rho.meyering.net>

On Mon, Sep 17, 2007 at 01:56:04PM +0200, Jim Meyering wrote:
> +++ b/test/Makefile.in
> @@ -0,0 +1,75 @@
> +#TEST_OPTS=--verbose --debug
> +SHELL_PATH ?= $(SHELL)
> +TAR ?= $(TAR)
> +RM ?= rm -f
> +
> +subdir := $(shell pwd|sed 's,.*/,,')
> +
> +srcdir = .
> +top_srcdir = ..
> +top_builddir = ..

I'd prefer to see this handled consistently across all our Makefiles.
Elsewhere we have srcdir = @srcdir@, for example.

> +dmdir = $(abs_top_srcdir)/../device-mapper
> +so_name = $(dmdir)/lib/ioctl/libdevmapper.so.1.02

Please avoid the 1.02 hard-coding:-)
(Include a symlink within the dm tree if you want at build time - maybe
from lib/libdevmapper.so -> ioctl/libdevmapper.so.*)

> +clean:
> +	rm -rf init.sh lvm-wrapper bin .bin-dir-stamp
> +
> +all: $(T)
> +.PHONY: $(T) clean

Last time I read the docs, they said .PHONY had to appear before
'clean:', not after it.

> +++ b/test/mkdtemp

Can these ugly infrastructure scripts be grouped together in a subdir?

> @@ -0,0 +1,107 @@
> +#!/bin/sh
> +# Create a temporary directory, sort of like mktemp -d does.
> +# Usage: mkdtemp /tmp phoey.XXXXXXXXXX

Where has this come from and has it been audited?  I get worried when I
see scripts like this that don't include any comments indicating that
security was the prime consideration, and doing our own audit would
be an unnecessary distraction.

I'd prefer not to have to ship (i.e. take responsibility for) a script
like this ourselves if we can avoid that.

> +++ b/test/t0000-basic.sh
> +lvm >/dev/null 2>&1

(How about using $LVM set to include the full path?)

> +# Protect ourselves from common misconfiguration to export
> +# CDPATH into the environment
> +unset CDPATH

I'd feel safer if this approach was inverted: clear all environment
variables except the few required for correct reproducible operation of
the tests.

The test preparation also needs to take steps to insulate itself
as far as possible from any 'real' lvm2 installation on the system.
This will also aid reproducibility.
E.g. I suggest it uses private directories instead of /etc/lvm and
/dev.

Alasdair
-- 
agk at redhat.com



  parent reply	other threads:[~2007-09-17 19:04 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-09-14 18:55 RFC: testing framework Jim Meyering
2007-09-14 23:11 ` Jun'ichi Nomura
2007-09-14 23:14   ` Jun'ichi Nomura
2007-09-17 10:35     ` Jim Meyering
2007-09-17 11:56 ` Jim Meyering
2007-09-17 18:15   ` Alasdair G Kergon
2007-09-17 19:04   ` Alasdair G Kergon [this message]
2007-09-17 20:55     ` Jim Meyering
2007-09-17 21:45       ` Alasdair G Kergon
2007-09-18 13:03     ` Jim Meyering

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=20070917190424.GL18444@agk.fab.redhat.com \
    --to=agk@redhat.com \
    --cc=lvm-devel@redhat.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.