| Message ID | f1acb2a0dfb39a4ff047d721edde821e9e296e72.1576583819.git.liu.denton@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | t: replace incorrect test_must_fail usage (part 1) | expand |
Denton Liu <liu.denton@gmail.com> writes: > We had named the parameters in attr_check() but $2 was being used > instead of $expect. Make all variable accesses in attr_check() use named > variables instead of numbered arguments for clarity. > > While we're at it, add variable assignments to the &&-chain. These > aren't ever expected to fail but for stylistic purposes, include them > anyway for stylistic purposes. That is OK but do so like this instead for brevity? attr_check () { - path="$1" expect="$2" + path="$1" expect="$2" git_opts="$3" && Giving (temporary) names to incoming parameters is so common that such a convention to save vertical screen real estate may be worth it. It is sad that you can pass only $IFS-free things in $3 due to the way it is used---perhaps it is sufficient for the purpose of tests in this script, but it still looks sad (just a comment without anything that is actionable). > Signed-off-by: Denton Liu <liu.denton@gmail.com> > --- > t/t0003-attributes.sh | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh > index 71e63d8b50..c47d4cfbcd 100755 > --- a/t/t0003-attributes.sh > +++ b/t/t0003-attributes.sh > @@ -5,19 +5,20 @@ test_description=gitattributes > . ./test-lib.sh > > attr_check () { > - path="$1" expect="$2" > + path="$1" && > + expect="$2" && > + git_opts="$3" && > > - git $3 check-attr test -- "$path" >actual 2>err && > - echo "$path: test: $2" >expect && > + git $git_opts check-attr test -- "$path" >actual 2>err && > + echo "$path: test: $expect" >expect && > test_cmp expect actual && > test_line_count = 0 err > } > > attr_check_quote () { > - > - path="$1" > - quoted_path="$2" > - expect="$3" > + path="$1" && > + quoted_path="$2" && > + expect="$3" && > > git check-attr test -- "$path" >actual && > echo "\"$quoted_path\": test: $expect" >expect &&
On Tue, Dec 17, 2019 at 7:01 AM Denton Liu <liu.denton@gmail.com> wrote: > We had named the parameters in attr_check() but $2 was being used > instead of $expect. Make all variable accesses in attr_check() use named > variables instead of numbered arguments for clarity. > > While we're at it, add variable assignments to the &&-chain. These > aren't ever expected to fail but for stylistic purposes, include them > anyway for stylistic purposes. As a justification, "stylistic purposes" isn't very strong. However, a solid justification is that keeping the &&-chain intact even for these assignments means that if someone comes along in the future and inserts code _above_ the assignments, and if that code could fail in some way, then the intact &&-chain will ensure that that failure is noticed rather than going unnoticed. > Signed-off-by: Denton Liu <liu.denton@gmail.com>
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh index 71e63d8b50..c47d4cfbcd 100755 --- a/t/t0003-attributes.sh +++ b/t/t0003-attributes.sh @@ -5,19 +5,20 @@ test_description=gitattributes . ./test-lib.sh attr_check () { - path="$1" expect="$2" + path="$1" && + expect="$2" && + git_opts="$3" && - git $3 check-attr test -- "$path" >actual 2>err && - echo "$path: test: $2" >expect && + git $git_opts check-attr test -- "$path" >actual 2>err && + echo "$path: test: $expect" >expect && test_cmp expect actual && test_line_count = 0 err } attr_check_quote () { - - path="$1" - quoted_path="$2" - expect="$3" + path="$1" && + quoted_path="$2" && + expect="$3" && git check-attr test -- "$path" >actual && echo "\"$quoted_path\": test: $expect" >expect &&
We had named the parameters in attr_check() but $2 was being used instead of $expect. Make all variable accesses in attr_check() use named variables instead of numbered arguments for clarity. While we're at it, add variable assignments to the &&-chain. These aren't ever expected to fail but for stylistic purposes, include them anyway for stylistic purposes. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- t/t0003-attributes.sh | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)