On Wed, Mar 25, 2020 at 1:55 AM Denton Liu <liu.denton@xxxxxxxxx> 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...
> Signed-off-by: Denton Liu <liu.denton@xxxxxxxxx>
> ---
> 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.
> + 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
> '
