| Message ID | 674de50db28a50554d7af6e5c869c427d06f78aa.1585115341.git.liu.denton@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | t: replace incorrect test_must_fail usage (part 3) | expand |
On Wed, Mar 25, 2020 at 1:55 AM Denton Liu <liu.denton@gmail.com> 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@gmail.com> > --- > 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 > '
Denton Liu <liu.denton@gmail.com> writes: > +generate_references () { > + for i Is it just me who thinks variables used in iteration should be called i, j, etc. only when they are integers? > + do > + oid=$(git rev-parse "$i") || return 1 > + printf '%s\t%s\n' "$oid" "$i" > + done > +} > + > test_expect_success setup ' > >file && > git add file && > @@ -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 && Hmph, can we avoid overlong lines like this one? generate_references >expect <<-EOF refs/tags/mark refs/tags/mark1.1 refs/tags/mark1.2 refs/tags/mark1.10 EOF i.e. teaching the helper function to read from its standard input stream, may make it more readable (i.e. it is more obvious what the order of expected output lines are, as you are listing them one by one on a line of its own).
diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh index 08b98f12b8..62d02152c7 100755 --- a/t/t5512-ls-remote.sh +++ b/t/t5512-ls-remote.sh @@ -4,6 +4,14 @@ test_description='git ls-remote' . ./test-lib.sh +generate_references () { + for i + do + oid=$(git rev-parse "$i") || return 1 + printf '%s\t%s\n' "$oid" "$i" + done +} + test_expect_success setup ' >file && git add file && @@ -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 && git ls-remote --sort="version:refname" --tags self >actual && test_cmp expect actual ' test_expect_success 'ls-remote --sort="-version:refname" --tags self' ' - cat >expect <<-EOF && - $(git rev-parse mark1.10) refs/tags/mark1.10 - $(git rev-parse mark1.2) refs/tags/mark1.2 - $(git rev-parse mark1.1) refs/tags/mark1.1 - $(git rev-parse mark) refs/tags/mark - EOF + generate_references refs/tags/mark1.10 refs/tags/mark1.2 refs/tags/mark1.1 refs/tags/mark >expect && git ls-remote --sort="-version:refname" --tags self >actual && test_cmp expect actual ' test_expect_success 'ls-remote --sort="-refname" --tags self' ' - cat >expect <<-EOF && - $(git rev-parse mark1.2) refs/tags/mark1.2 - $(git rev-parse mark1.10) refs/tags/mark1.10 - $(git rev-parse mark1.1) refs/tags/mark1.1 - $(git rev-parse mark) refs/tags/mark - EOF + generate_references refs/tags/mark1.2 refs/tags/mark1.10 refs/tags/mark1.1 refs/tags/mark >expect && git ls-remote --sort="-refname" --tags self >actual && test_cmp expect actual ' @@ -212,17 +205,16 @@ test_expect_success 'protocol v2 supports hiderefs' ' test_expect_success 'ls-remote --symref' ' git fetch origin && - cat >expect <<-EOF && - ref: refs/heads/master HEAD - $(git rev-parse HEAD) HEAD - $(git rev-parse refs/heads/master) refs/heads/master - $(git rev-parse HEAD) refs/remotes/origin/HEAD - $(git rev-parse refs/remotes/origin/master) refs/remotes/origin/master - $(git rev-parse refs/tags/mark) refs/tags/mark - $(git rev-parse refs/tags/mark1.1) refs/tags/mark1.1 - $(git rev-parse refs/tags/mark1.10) refs/tags/mark1.10 - $(git rev-parse refs/tags/mark1.2) refs/tags/mark1.2 - EOF + echo "ref: refs/heads/master HEAD" >expect && + generate_references HEAD \ + refs/heads/master >>expect && + oid=$(git rev-parse HEAD) && + echo "$oid refs/remotes/origin/HEAD" >>expect && + generate_references refs/remotes/origin/master \ + refs/tags/mark \ + refs/tags/mark1.1 \ + refs/tags/mark1.10 \ + refs/tags/mark1.2 >>expect && # Protocol v2 supports sending symrefs for refs other than HEAD, so use # protocol v0 here. GIT_TEST_PROTOCOL_VERSION=0 git ls-remote --symref >actual &&
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 it. Replace these here-docs with generate_references(), which actually reports when `git rev-parse` fails. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- t/t5512-ls-remote.sh | 50 +++++++++++++++++++------------------------- 1 file changed, 21 insertions(+), 29 deletions(-)