| Message ID | 326fb7965036fccc1c23ad02f200251f6388e6dd.1711028473.git.ps@pks.im (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | t7800: fix quoting of looped test bodies | expand |
Patrick Steinhardt <ps@pks.im> writes: > In some cases it makes sense to loop around test cases so that we can > execute the same test with slightly different arguments. There are some > gotchas around quoting here though that are easy to miss and that may > lead to easy-to-miss errors and portability issues. > > Document the proper way to do this in "t/README". > > Signed-off-by: Patrick Steinhardt <ps@pks.im> > --- > t/README | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/t/README b/t/README > index 36463d0742..d56401a254 100644 > --- a/t/README > +++ b/t/README > @@ -721,6 +721,25 @@ The "do's:" > Note that we still &&-chain the loop to propagate failures from > earlier commands. > > + - Repeat tests with slightly different arguments in a loop. > + > + In some cases it may make sense to re-run the same set of tests with > + different options or commands to ensure that the command behaves > + despite the different parameters. This can be achieved by looping > + around a specific parameter: > + > + for arg in '' "--foo" > + do > + test_expect_success "test command ${arg:-without arguments}" ' > + command $arg > + ' > + done > + > + Note that while the test title uses double quotes ("), the test body > + should continue to use single quotes ('). The loop variable will be > + accessible regardless of the single quotes as the test body is passed > + to `eval`. We also want to say that they are not equivalent, don't we? for var in '' a 'b"c' do test_expect_success "with dq <$var>" " echo \"$var\" " done breaks, but if we use test_expect_success "with sq <$var>" ' echo "$var" ' in the loop, it works as expected. Other than that, all three patches do make sense. Thanks.
On Thu, Mar 21, 2024 at 10:12:40AM -0700, Junio C Hamano wrote: > Patrick Steinhardt <ps@pks.im> writes: > > > In some cases it makes sense to loop around test cases so that we can > > execute the same test with slightly different arguments. There are some > > gotchas around quoting here though that are easy to miss and that may > > lead to easy-to-miss errors and portability issues. > > > > Document the proper way to do this in "t/README". > > > > Signed-off-by: Patrick Steinhardt <ps@pks.im> > > --- > > t/README | 19 +++++++++++++++++++ > > 1 file changed, 19 insertions(+) > > > > diff --git a/t/README b/t/README > > index 36463d0742..d56401a254 100644 > > --- a/t/README > > +++ b/t/README > > @@ -721,6 +721,25 @@ The "do's:" > > Note that we still &&-chain the loop to propagate failures from > > earlier commands. > > > > + - Repeat tests with slightly different arguments in a loop. > > + > > + In some cases it may make sense to re-run the same set of tests with > > + different options or commands to ensure that the command behaves > > + despite the different parameters. This can be achieved by looping > > + around a specific parameter: > > + > > + for arg in '' "--foo" > > + do > > + test_expect_success "test command ${arg:-without arguments}" ' > > + command $arg > > + ' > > + done > > + > > + Note that while the test title uses double quotes ("), the test body > > + should continue to use single quotes ('). The loop variable will be > > + accessible regardless of the single quotes as the test body is passed > > + to `eval`. > > We also want to say that they are not equivalent, don't we? > > for var in '' a 'b"c' > do > test_expect_success "with dq <$var>" " > echo \"$var\" > " > done > > breaks, but if we use > > test_expect_success "with sq <$var>" ' > echo "$var" > ' > > in the loop, it works as expected. Hum, good point. How about the below diff? Will reroll the patch series if that looks good to you. --- a/t/README +++ b/t/README @@ -736,7 +736,8 @@ The "do's:" done Note that while the test title uses double quotes ("), the test body - should continue to use single quotes ('). The loop variable will be + should continue to use single quotes (') to avoid breakage in case the + values contain e.g. quoting characters. The loop variable will be accessible regardless of the single quotes as the test body is passed to `eval`. Patrick
Patrick Steinhardt <ps@pks.im> writes: > Hum, good point. How about the below diff? Will reroll the patch series > if that looks good to you. > > --- a/t/README > +++ b/t/README > @@ -736,7 +736,8 @@ The "do's:" > done > > Note that while the test title uses double quotes ("), the test body > - should continue to use single quotes ('). The loop variable will be > + should continue to use single quotes (') to avoid breakage in case the > + values contain e.g. quoting characters. The loop variable will be > accessible regardless of the single quotes as the test body is passed > to `eval`. Wow, simple and effective.
diff --git a/t/README b/t/README index 36463d0742..d56401a254 100644 --- a/t/README +++ b/t/README @@ -721,6 +721,25 @@ The "do's:" Note that we still &&-chain the loop to propagate failures from earlier commands. + - Repeat tests with slightly different arguments in a loop. + + In some cases it may make sense to re-run the same set of tests with + different options or commands to ensure that the command behaves + despite the different parameters. This can be achieved by looping + around a specific parameter: + + for arg in '' "--foo" + do + test_expect_success "test command ${arg:-without arguments}" ' + command $arg + ' + done + + Note that while the test title uses double quotes ("), the test body + should continue to use single quotes ('). The loop variable will be + accessible regardless of the single quotes as the test body is passed + to `eval`. + And here are the "don'ts:"
In some cases it makes sense to loop around test cases so that we can execute the same test with slightly different arguments. There are some gotchas around quoting here though that are easy to miss and that may lead to easy-to-miss errors and portability issues. Document the proper way to do this in "t/README". Signed-off-by: Patrick Steinhardt <ps@pks.im> --- t/README | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)