You might be right. That sounds like a nice incremental improvement.
On Mon, Aug 20, 2012 at 01:12:14PM -0700, Ethan Jackson wrote: > Almost all of these are of the form ON_EXIT(['kill `cat pidfile`]) In > order to increase readability and avoid typos, does it make sense to > add an additional for of the macro ON_EXIT_KILL([pidfile]). > > Just a thought, doesn't matter that much. > Ethan > > On Mon, Aug 20, 2012 at 11:42 AM, Mehak Mahajan <mmaha...@nicira.com> wrote: > > Looks good. > > > > Thanx! > > Mehak > > > > > > On Mon, Aug 20, 2012 at 10:33 AM, Ben Pfaff <b...@nicira.com> wrote: > >> > >> Several of the tests start daemons and then need to make sure that the > >> daemons get killed when the test completes, even if it completes in the > >> middle due to an early failure. Until now, they have been using manual > >> shell "trap" calls to do this. This works well enough for simple cases, > >> but sometimes multiple macros start daemons in a single test, and then > >> each "trap" has to be carefully written to kill off the daemons for the > >> previously invoked macros. > >> > >> This commit introduces a new macro ON_EXIT whose use is composable: each > >> call appends a new action to the ones already specified. > >> > >> Signed-off-by: Ben Pfaff <b...@nicira.com> > >> --- > >> tests/ofproto-macros.at | 5 ++--- > >> tests/ofproto.at | 2 +- > >> tests/ovs-monitor-ipsec.at | 2 +- > >> tests/ovs-vsctl.at | 2 +- > >> tests/ovs-xapi-sync.at | 2 +- > >> tests/testsuite.at | 11 +++++++++++ > >> tests/unixctl-py.at | 3 +-- > >> tests/vlog.at | 9 ++++----- > >> 8 files changed, 22 insertions(+), 14 deletions(-) > >> > >> diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at > >> index 944fbac..52f19fc 100644 > >> --- a/tests/ofproto-macros.at > >> +++ b/tests/ofproto-macros.at > >> @@ -33,7 +33,7 @@ m4_define([OVS_VSWITCHD_START], > >> OVS_LOGDIR=`pwd`; export OVS_LOGDIR > >> OVS_DBDIR=`pwd`; export OVS_DBDIR > >> OVS_SYSCONFDIR=`pwd`; export OVS_SYSCONFDIR > >> - trap 'kill `cat ovsdb-server.pid ovs-vswitchd.pid`' 0 > >> + ON_EXIT([kill `cat ovsdb-server.pid ovs-vswitchd.pid`]) > >> > >> dnl Create database. > >> touch .conf.db.~lock~ > >> @@ -65,5 +65,4 @@ m4_define([OVS_VSWITCHD_START], > >> > >> m4_define([OVS_VSWITCHD_STOP], > >> [AT_CHECK([ovs-appctl -t ovs-vswitchd exit]) > >> - AT_CHECK([ovs-appctl -t ovsdb-server exit]) > >> - trap '' 0]) > >> + AT_CHECK([ovs-appctl -t ovsdb-server exit])]) > >> diff --git a/tests/ofproto.at b/tests/ofproto.at > >> index e3239f9..47402f7 100644 > >> --- a/tests/ofproto.at > >> +++ b/tests/ofproto.at > >> @@ -910,7 +910,7 @@ echo n_msgs=$n_msgs > >> OVS_VSWITCHD_START > >> > >> # Start a monitor watching the flow table, then make it block. > >> -trap 'kill `cat ovsdb-server.pid ovs-vswitchd.pid ovs-ofctl.pid`' 0 > >> +ON_EXIT([kill `cat ovs-ofctl.pid`]) > >> ovs-ofctl monitor br0 watch: --detach --no-chdir --pidfile >monitor.log > >> 2>&1 > >> AT_CAPTURE_FILE([monitor.log]) > >> ovs-appctl -t ovs-ofctl ofctl/block > >> diff --git a/tests/ovs-monitor-ipsec.at b/tests/ovs-monitor-ipsec.at > >> index 1a83161..e66c943 100644 > >> --- a/tests/ovs-monitor-ipsec.at > >> +++ b/tests/ovs-monitor-ipsec.at > >> @@ -8,7 +8,7 @@ OVS_DBDIR=`pwd`; export OVS_DBDIR > >> OVS_PKGDATADIR=`pwd`; export OVS_PKGDATADIR > >> cp "$top_srcdir/vswitchd/vswitch.ovsschema" . > >> > >> -trap 'kill `cat pid ovs-monitor-ipsec.pid`' 0 > >> +ON_EXIT([kill `cat pid ovs-monitor-ipsec.pid`]) > >> > >> mkdir etc etc/init.d etc/racoon etc/racoon/certs > >> mkdir usr usr/sbin > >> diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at > >> index ef2c0c0..84cc272 100644 > >> --- a/tests/ovs-vsctl.at > >> +++ b/tests/ovs-vsctl.at > >> @@ -741,7 +741,7 @@ AT_CLEANUP > >> > >> AT_SETUP([database commands -- conditions]) > >> AT_KEYWORDS([ovs-vsctl]) > >> -trap 'kill `cat pid`' 0 > >> +ON_EXIT([kill `cat pid`]) > >> OVS_VSCTL_SETUP > >> AT_CHECK( > >> [RUN_OVS_VSCTL_TOGETHER( > >> diff --git a/tests/ovs-xapi-sync.at b/tests/ovs-xapi-sync.at > >> index 25acc74..29d4737 100644 > >> --- a/tests/ovs-xapi-sync.at > >> +++ b/tests/ovs-xapi-sync.at > >> @@ -16,7 +16,7 @@ cp "$top_srcdir/vswitchd/vswitch.ovsschema" . > >> cp "$top_srcdir/xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync" \ > >> ovs-xapi-sync > >> > >> -trap 'kill `cat pid ovs-xapi-sync.pid`' 0 > >> +ON_EXIT([kill `cat pid ovs-xapi-sync.pid`]) > >> > >> mkdir var var/run > >> touch var/run/xapi_init_complete.cookie > >> diff --git a/tests/testsuite.at b/tests/testsuite.at > >> index e8f7498..2b4ccdb 100644 > >> --- a/tests/testsuite.at > >> +++ b/tests/testsuite.at > >> @@ -53,6 +53,17 @@ m4_define([OVS_WAIT_UNTIL], [OVS_WAIT([$1], [$2])]) > >> m4_define([OVS_WAIT_WHILE], > >> [OVS_WAIT([if $1; then return 1; else return 0; fi], [$2])]) > >> > >> +dnl ON_EXIT([COMMANDS]) > >> +dnl > >> +dnl Adds the shell COMMANDS to a collection executed when the current > >> test > >> +dnl completes, as a cleanup action. (The most common use is to kill a > >> +dnl daemon started by the test. This is important to prevent tests that > >> +dnl start daemons from hanging at exit.) > >> +m4_define([ON_EXIT], [trap '. ./cleanup' 0; cat >>cleanup <<'EOF' > >> +$1 > >> +EOF > >> +]) > >> + > >> m4_include([tests/ovsdb-macros.at]) > >> m4_include([tests/ofproto-macros.at]) > >> > >> diff --git a/tests/unixctl-py.at b/tests/unixctl-py.at > >> index 15fd86b..b54d409 100644 > >> --- a/tests/unixctl-py.at > >> +++ b/tests/unixctl-py.at > >> @@ -94,7 +94,7 @@ OVS_RUNDIR=`pwd`; export OVS_RUNDIR > >> OVS_LOGDIR=`pwd`; export OVS_LOGDIR > >> OVS_DBDIR=`pwd`; export OVS_DBDIR > >> OVS_SYSCONFDIR=`pwd`; export OVS_SYSCONFDIR > >> -trap 'kill `cat test-unixctl.py.pid`' 0 > >> +ON_EXIT([kill `cat test-unixctl.py.pid`]) > >> AT_CAPTURE_FILE([`pwd`/test-unixctl.py.log]) > >> AT_CHECK([$PYTHON $srcdir/test-unixctl.py --log-file --pidfile --detach]) > >> > >> @@ -159,7 +159,6 @@ sed 's/ovs-appctl/appctl.py/' stderr > experr > >> AT_CHECK([PYAPPCTL -t test-unixctl.py bogus], [2], [], [experr]) > >> > >> AT_CHECK([APPCTL -t test-unixctl.py exit]) > >> -trap '' 0] > >> AT_CLEANUP > >> > >> > >> diff --git a/tests/vlog.at b/tests/vlog.at > >> index a8a947c..957d872 100644 > >> --- a/tests/vlog.at > >> +++ b/tests/vlog.at > >> @@ -109,7 +109,7 @@ OVS_RUNDIR=`pwd`; export OVS_RUNDIR > >> OVS_LOGDIR=`pwd`; export OVS_LOGDIR > >> OVS_DBDIR=`pwd`; export OVS_DBDIR > >> OVS_SYSCONFDIR=`pwd`; export OVS_SYSCONFDIR > >> -trap 'kill `cat test-unixctl.py.pid`' 0 > >> +ON_EXIT([kill `cat test-unixctl.py.pid`]) > >> > >> AT_CAPTURE_FILE([log]) > >> AT_CAPTURE_FILE([log.old]) > >> @@ -121,7 +121,6 @@ AT_CHECK([APPCTL -t test-unixctl.py log message2]) > >> AT_CHECK([APPCTL -t test-unixctl.py vlog/reopen]) > >> AT_CHECK([APPCTL -t test-unixctl.py log message3]) > >> AT_CHECK([APPCTL -t test-unixctl.py exit]) > >> -trap '' 0 > >> > >> AT_CHECK([sed 's/.*|//' log.old], [0], [dnl > >> Entering run loop. > >> @@ -139,7 +138,7 @@ OVS_RUNDIR=`pwd`; export OVS_RUNDIR > >> OVS_LOGDIR=`pwd`; export OVS_LOGDIR > >> OVS_DBDIR=`pwd`; export OVS_DBDIR > >> OVS_SYSCONFDIR=`pwd`; export OVS_SYSCONFDIR > >> -trap 'kill `cat test-unixctl.py.pid`' 0 > >> +ON_EXIT([kill `cat test-unixctl.py.pid`]) > >> > >> AT_CHECK([$PYTHON $srcdir/test-unixctl.py --pidfile --detach]) > >> > >> @@ -162,7 +161,7 @@ OVS_RUNDIR=`pwd`; export OVS_RUNDIR > >> OVS_LOGDIR=`pwd`; export OVS_LOGDIR > >> OVS_DBDIR=`pwd`; export OVS_DBDIR > >> OVS_SYSCONFDIR=`pwd`; export OVS_SYSCONFDIR > >> -trap 'kill `cat test-unixctl.py.pid`' 0 > >> +ON_EXIT([kill `cat test-unixctl.py.pid`]) > >> > >> AT_CHECK([$PYTHON $srcdir/test-unixctl.py --log-file=`pwd`/log --pidfile > >> --detach]) > >> AT_CHECK([APPCTL -t test-unixctl.py log message]) > >> @@ -189,7 +188,7 @@ OVS_RUNDIR=`pwd`; export OVS_RUNDIR > >> OVS_LOGDIR=`pwd`; export OVS_LOGDIR > >> OVS_DBDIR=`pwd`; export OVS_DBDIR > >> OVS_SYSCONFDIR=`pwd`; export OVS_SYSCONFDIR > >> -trap 'kill `cat test-unixctl.py.pid`' 0 > >> +ON_EXIT([kill `cat test-unixctl.py.pid`]) > >> > >> AT_CAPTURE_FILE([log]) > >> AT_CHECK([$PYTHON $srcdir/test-unixctl.py --log-file=`pwd`/log --pidfile > >> --detach]) > >> -- > >> 1.7.2.5 > >> > >> _______________________________________________ > >> dev mailing list > >> dev@openvswitch.org > >> http://openvswitch.org/mailman/listinfo/dev > > > > > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > http://openvswitch.org/mailman/listinfo/dev > > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
