| Message ID | 1443550635-29312-1-git-send-email-blp@nicira.com |
|---|---|
| State | Accepted |
| Headers | show |
Hi Ben, On Tue, Sep 29, 2015 at 11:17:13AM -0700, Ben Pfaff wrote: > AT_CHECK runs its commands in a subshell. That means that (among other > effects), any variable assignments within its commands will disappear after > the commands' completion. That doesn't matter for any of the existing > users, which don't do the sorts of things that affect an outer shell > environment anyhow, but an upcoming user wants to make a shell assignment > that persists. This commit makes that possible, by using AT_CHECK > (actually AT_FAIL_IF but it's moot) only upon failure instead of bracketing > the entire test. > > Signed-off-by: Ben Pfaff <blp@nicira.com> From my point of view adding support for side effects isn't particularly desirable, however, I'm not sure that I know a way to cleanly handle the use-case in the third patch of the series. So I guess its best to give a little to get a lot. With the above in mind, the entire series: Reviewed-by: Simon Horman <simon.horman@netronome.com>
On Tue, Sep 29, 2015 at 11:17:13AM -0700, Ben Pfaff wrote: > AT_CHECK runs its commands in a subshell. That means that (among other > effects), any variable assignments within its commands will disappear after > the commands' completion. That doesn't matter for any of the existing > users, which don't do the sorts of things that affect an outer shell > environment anyhow, but an upcoming user wants to make a shell assignment > that persists. This commit makes that possible, by using AT_CHECK > (actually AT_FAIL_IF but it's moot) only upon failure instead of bracketing > the entire test. > > Signed-off-by: Ben Pfaff <blp@nicira.com> > --- I like the idea of passing variables. Patch looks good, nothing breaks here though I had to manually update the couple patches here. Acked-by: Flavio Leitner <fbl@sysclose.org>
On Wed, Nov 25, 2015 at 04:21:22PM +0900, Simon Horman wrote: > Hi Ben, > > On Tue, Sep 29, 2015 at 11:17:13AM -0700, Ben Pfaff wrote: > > AT_CHECK runs its commands in a subshell. That means that (among other > > effects), any variable assignments within its commands will disappear after > > the commands' completion. That doesn't matter for any of the existing > > users, which don't do the sorts of things that affect an outer shell > > environment anyhow, but an upcoming user wants to make a shell assignment > > that persists. This commit makes that possible, by using AT_CHECK > > (actually AT_FAIL_IF but it's moot) only upon failure instead of bracketing > > the entire test. > > > > Signed-off-by: Ben Pfaff <blp@nicira.com> > > From my point of view adding support for side effects isn't particularly > desirable, however, I'm not sure that I know a way to cleanly handle the > use-case in the third patch of the series. So I guess its best to give a > little to get a lot. > > With the above in mind, the entire series: > > Reviewed-by: Simon Horman <simon.horman@netronome.com> I guess I feel like scoping in a shell environment is more of a surprise than a feature. Thanks Simon and Flavio! I applied these three patches to master.
diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at index 058830b..535d923 100644 --- a/tests/ovs-macros.at +++ b/tests/ovs-macros.at @@ -31,13 +31,14 @@ ovs_wait () { # in the normal case. POSIX doesn't require fractional times to # work, so this might not work. sleep 0.1 - ovs_wait_cond && exit 0 + ovs_wait_cond && return 0 + # Then wait up to 10 seconds. for d in 0 1 2 3 4 5 6 7 8 9; do sleep 1 - ovs_wait_cond && exit 0 + ovs_wait_cond && return 0 done - exit 1 + return 1 } # Prints the integers from $1 to $2, increasing by $3 (default 1) on stdout. @@ -87,11 +88,16 @@ fi ] m4_divert_pop([PREPARE_TESTS]) -m4_define([OVS_WAIT], - [AT_CHECK( - [ovs_wait_cond () { $1 +m4_define([OVS_WAIT], [dnl +ovs_wait_cond () { + $1 } -ovs_wait], [0], [ignore], [ignore], [$2])]) +if ovs_wait; then : +else + $2 + AT_FAIL_IF([:]) +fi +]) 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])])
AT_CHECK runs its commands in a subshell. That means that (among other effects), any variable assignments within its commands will disappear after the commands' completion. That doesn't matter for any of the existing users, which don't do the sorts of things that affect an outer shell environment anyhow, but an upcoming user wants to make a shell assignment that persists. This commit makes that possible, by using AT_CHECK (actually AT_FAIL_IF but it's moot) only upon failure instead of bracketing the entire test. Signed-off-by: Ben Pfaff <blp@nicira.com> --- tests/ovs-macros.at | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)