Re: [PATCH 2/8] t5512: generate references with generate_references()
From: Eric Sunshine <hidden>
Date: 2020-03-25 06:08:46
On Wed, Mar 25, 2020 at 1:55 AM Denton Liu [off-list ref] wrote:
t5512: generate references with generate_references()
This summary doesn't say anything useful. How about this instead?
t5512: stop losing git exit code in here-docs
The expected references are generated using a here-doc with some inline subshells. If one of the `git rev-parse` invocations within the subshells failed, its return code is swallowed and we won't know about
s/failed/fails/
it. Replace these here-docs with generate_references(), which actually reports when `git rev-parse` fails.
A couple nits below...
quoted hunk ↗ jump to hunk
Signed-off-by: Denton Liu <redacted> ---diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh@@ -4,6 +4,14 @@ test_description='git ls-remote' +generate_references () { + for i + do + oid=$(git rev-parse "$i") || return 1 + printf '%s\t%s\n' "$oid" "$i"
I think the more usual way to say this in our test suite would be:
oid=$(git rev-parse "$i") &&
printf '%s\t%s\n' "$oid" "$i" || return 1
which has the nice property that someone can come along and insert
additional code in the loop body before the final "|| return 1"
without having to spend a lot of time trying to work out if the
&&-chain is intact or broken.
quoted hunk ↗ jump to hunk
+ done +}@@ -43,34 +51,19 @@ test_expect_success 'ls-remote self' ' test_expect_success 'ls-remote --sort="version:refname" --tags self' ' - cat >expect <<-EOF && - $(git rev-parse mark) refs/tags/mark - $(git rev-parse mark1.1) refs/tags/mark1.1 - $(git rev-parse mark1.2) refs/tags/mark1.2 - $(git rev-parse mark1.10) refs/tags/mark1.10 - EOF + generate_references refs/tags/mark refs/tags/mark1.1 refs/tags/mark1.2 refs/tags/mark1.10 >expect &&
This gets awfully wide and loses some readability. Perhaps:
generate_references \
refs/tags/mark \
refs/tags/mark1.1 \
refs/tags/mark1.2 \
refs/tags/mark1.10 >expect &&
git ls-remote --sort="version:refname" --tags self >actual &&
test_cmp expect actual
'