patch, 7 partst1005: assert output of ls-files

I've noticed that several tests in t9001-send-email.sh don't use the files created from redirecting output of git commands. So I wrote a crude script to find similar issues in other tests:

    from sys import argv
    from sys import exit
    import re
    
    script = argv[1]
    
    filename_pattern = re.compile('([-a-z_A-Z]+|&1)')
    git_with_output_pattern = re.compile('git [^&"]*[ 2][>](?!/)')
    
    while True:
        res = git_with_output_pattern.search(script)
        if res is None:
            break
        filename_index = res.span()[1]
        res = filename_pattern.search(script[filename_index:])
        filename = res.group()
    
        script = script[filename_index + len(filename):]
    
        if filename == '&1':
            continue
    
        read_index = script.find(filename)
        if read_index < 0:
            print("File '" + filename + "' is unused")
            print("Script: ")
            print(script)
            exit(1)
        script = script[read_index + len(filename):]

It doesn't check the tests very throughly and has a lot of false-positives, but this is enough for now. I invoke it from test_expect_success() like so:

---- 8< ----

Test 'reset should work' in t1005-read-tree-reset.sh compares two files "expect" and "actual" to assert the expected output of "git ls-files". Several other tests in the same file also create files "expect" and "actual", but don't use them in assertions.

Assert output of "git ls-files" in t1005-read-tree-reset.sh to improve test coverage.

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t1005-read-tree-reset.sh | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

Andrei Rybak venit, vidit, dixit 2023-03-12 21:15:13:

Just in case someone else was wondering, too:

All these subtests write to `expect` just before the provided context lines, so there indeed is something to compare to, and it is the output of `git ls-files -s` before any changes and resets. As a consequence, these subtests checked only removal of remnants in the woring tree before the patch, and they check removal of index bits after the patch (additionally).

Looks fine to me - though one could probably use `git ls-files -s -o` or such instead, the suggested version is more "explicit".

Michael

On 14/03/2023 09:51, Michael J Gruber wrote:

Thank you for reviewing this.

None the tests (both in t1005 and other files) use this combination of flags for `ls-files` right now. Checked using:

     $ git grep 'ls-files.*[^a-z-][-][os][^|]*[^a-z-][-][so]' || echo none
     none
     $ git grep -E 'ls-files .* --(stage|others)'
     Documentation/git-ls-files.txt:'git ls-files --unmerged' and 'git ls-files --stage' can be used to examine
     contrib/hg-to-git/hg-to-git.py:    os.system('git ls-files -x .hg --others | git update-index --add --stdin')
     t/t1092-sparse-checkout-compatibility.sh:       git -C sparse-index ls-files --sparse --stage >cache &&
     t/t1092-sparse-checkout-compatibility.sh:       git -C sparse-index ls-files --sparse --stage >cache &&
     t/t1306-xdg-files.sh:    git ls-files --exclude-standard --ignored --others >actual) &&

In this topic, I plan to focus on just fixes of unused files and to leave flags of `git ls-files` as is for now.

Tests in t1507-rev-parse-upstream.sh compare files "expect" and "actual" to assert the output of "git rev-parse", "git show", and "git log". However, two of the tests '@{reflog}-parsing does not look beyond colon' and '@{upstream}-parsing does not look beyond colon' don't inspect the contents of the created files.

Assert output of "git rev-parse" in tests in t1507-rev-parse-upstream.sh to improve test coverage.

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t1507-rev-parse-upstream.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

I've sent this patch twice by accident.  Please ignore this version.

The correct version has subject "[PATCH v1 7/7] t1507: assert output of rev-parse". https://lore.kernel.org/git/20230312201520.370234-9-rybak.a.v@gmail.com/

Test "cat-file $arg1 $arg2 error on missing full OID" in t1006-cat-file.sh compares files "expect.err" and "err.actual" to assert the expected error output of "git cat-file". A similar test in the same file named "cat-file $arg1 $arg2 error on missing short OID" also creates these two files, but doesn't use them in assertions.

Assert error output of "git cat-file" in test "cat-file $arg1 $arg2 error on missing short OID" of t1006-cat-file.sh to improve test coverage.

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t1006-cat-file.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Builtin "git mktree" writes the the object name of the tree object built to the standard output. Tests 'mktree refuses to read ls-tree -r output (1)' and 'mktree refuses to read ls-tree -r output (2)' in "t1010-mktree.sh" redirect output of "git mktree" to a file, but don't use its contents in assertions.

Assert that the output of "git mktree" is empty when it refuses to build a tree object.

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t1010-mktree.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Andrei Rybak <rybak.a.v@gmail.com> writes:

I am ambivalent. As long as a failing command signals its failure with its non-zero exit status value, the consumer of the output should not blindly use the output from such a failing command. Is there a strong reason why we want users rely on the command to be silent when it fails?

An obvious alternative is to stop producing "actual" file, and it might be a better idea; unless there is a good reason why we should expect the command to be silent, that is.

Thanks.

Test 'gitdir selection on unsupported repo' in t1302-repo-version.sh writes output of a "git config" invocation to file "actual". However, the test doesn't have any assertions for the file. The file was used by this test until commit b9605bc4f2 (config: only read .git/config from configured repos, 2016-09-12), before which "git config" was expected to print the bogus value of "core.repositoryformatversion" to standard output.

Don't redirect output of "git config" to file "actual" in test 'gitdir selection on unsupported repo'.

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t1302-repo-version.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

In t1400-update-ref.sh test 'transaction can create and delete' creates files "expect" and "actual", but doesn't compare them. Similarly, test 'transaction cannot restart ongoing transaction' redirects output of "git update-ref" to file "actual", but doesn't check its contents with any assertions.

Assert output of "git update-ref" in tests to improve test coverage in t1400-update-ref.sh.

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t1400-update-ref.sh | 3 +++
 1 file changed, 3 insertions(+)

Some tests in file t1404-update-ref-errors.sh create file "unchanged" as the expected side for a test_cmp assertion at the end of the test for output of "git for-each-ref". The filename conveys the expectation that the output won't change between two invocations of "git for-each-ref".

Test 'no bogus intermediate values during delete' also creates a file named "unchanged". However, in this test the reference is being deleted, i.e. it _does change_. The file itself isn't used for any assertions in the test.

Don't create the unused and slightly misleading file "unchanged".

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t1404-update-ref-errors.sh | 1 -
 1 file changed, 1 deletion(-)

Andrei Rybak <rybak.a.v@gmail.com> writes:

I think the name "unchanged" is a reference to: the state recorded in this file is before all the interesting changes done in this test.

So another for-each-ref, after the "lock, start a process that waits for and then removes the ref" begins but while the other process is still waiting, whose output is compared with "unchanged" may have been another way to perform this test, but we have "it could be $D that is what we want, and two plausible 'wrong' answers are $C and undefined" that is sufficient. So I agree with removing the line that creates "unchanged".

The other test to the file added by the same commit 6a2a7736 (t1404: demonstrate two problems with reference transactions, 2017-09-08) creates the "unchanged" file in the same way, but it does get used after running "update-ref" that is tested. I would not be surprised if the one removed by this patch was created by a cut-and-paste by mistake.

Thanks.

Tests in t1507-rev-parse-upstream.sh compare files "expect" and "actual" to assert the output of "git rev-parse", "git show", and "git log". However, two of the tests '@{reflog}-parsing does not look beyond colon' and '@{upstream}-parsing does not look beyond colon' don't inspect the contents of the created files.

Assert output of "git rev-parse" in tests in t1507-rev-parse-upstream.sh to improve test coverage.

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t1507-rev-parse-upstream.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Andrei Rybak <rybak.a.v@gmail.com> writes:

I think it is better not to insist that a failing 'mktree' is silent, and I think the filename "unchanged" is understandable and is unfair to call it "misleading" (but the patch itself to remove the line that creates the unused file makes perfect sense). Other than these two small nits, I found everything else in the series good changes.

Thanks for a pleasant read.

On 13/03/2023 23:41, Junio C Hamano wrote:

Thank you for review. I have limited bandwidth this week, but I'll try to send a v2 incorporating this feedback on the weekend.

Here's a v2 for various fixes of unused files in t0??? and t1???.
Original cover letter is at
  https://lore.kernel.org/git/20230312201520.370234-1-rybak.a.v@gmail.com/T/#m63071efd1e2f876fdcbd0c384130b0ec7859f885

Changes since v1 address Junio's review:

  - just don't redirect output of mktree in patch 3/7
  - rewritten commit message of patch 6/7, where I'd completely misunderstood
    the code of the tests.

Range diff:

1:  017f1d8173 = 1:  59a868c0b8 t1005: assert output of ls-files
2:  7a75864e00 = 2:  a3514687ad t1006: assert error output of cat-file
3:  e23a16e560 ! 3:  4cb07fa965 t1010: assert empty output of mktree
    @@ Metadata
     Author: Andrei Rybak <rybak.a.v@gmail.com>
     
      ## Commit message ##
    -    t1010: assert empty output of mktree
    +    t1010: don't create unused files
     
         Builtin "git mktree" writes the the object name of the tree object built
         to the standard output.  Tests 'mktree refuses to read ls-tree -r output
    @@ Commit message
         "t1010-mktree.sh" redirect output of "git mktree" to a file, but don't
         use its contents in assertions.
     
    -    Assert that the output of "git mktree" is empty when it refuses to build
    -    a tree object.
    +    Don't redirect output of "git mktree" to file "actual" in tests that
    +    assert that an invocation of "git mktree" must fail.
    +
    +    Output of "git mktree" is empty when it refuses to build a tree object.
    +    So, alternatively, the test could assert that the output is empty.
    +    However, there isn't a good reason for the user to expect the command to
    +    be silent in such cases, so we shouldn't enforce it.  The user shouldn't
    +    use the output of a failing command anyway.
     
         Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
     
    @@ t/t1010-mktree.sh: test_expect_success 'allow missing object with --missing' '
      
      test_expect_success 'mktree refuses to read ls-tree -r output (1)' '
     -  test_must_fail git mktree <all >actual
    -+  test_must_fail git mktree <all >actual &&
    -+  test_must_be_empty actual
    ++  test_must_fail git mktree <all
      '
      
      test_expect_success 'mktree refuses to read ls-tree -r output (2)' '
     -  test_must_fail git mktree <all.withsub >actual
    -+  test_must_fail git mktree <all.withsub >actual &&
    -+  test_must_be_empty actual
    ++  test_must_fail git mktree <all.withsub
      '
      
      test_done
4:  462cfa7025 = 4:  1ed6030a4f t1302: don't create unused file
5:  9fa04e479c = 5:  002942d81c t1400: assert output of update-ref
6:  e79566cc32 ! 6:  2e3446fc2f t1404: don't create unused file
    @@ Commit message
     
         Some tests in file t1404-update-ref-errors.sh create file "unchanged" as
         the expected side for a test_cmp assertion at the end of the test for
    -    output of "git for-each-ref".  The filename conveys the expectation that
    -    the output won't change between two invocations of "git for-each-ref".
    +    output of "git for-each-ref".  Test 'no bogus intermediate values during
    +    delete' also creates a file named "unchanged" using "git for-each-ref".
    +    However, the file isn't used for any assertions in the test.  Instead,
    +    "git rev-parse" is used to compare the reference with variable $D.
     
    -    Test 'no bogus intermediate values during delete' also creates a file
    -    named "unchanged".  However, in this test the reference is being
    -    deleted, i.e. it _does change_.  The file itself isn't used for any
    -    assertions in the test.
    -
    -    Don't create the unused and slightly misleading file "unchanged".
    +    Don't create unused file "unchanged" in test 'no bogus intermediate
    +    values during delete' of t1404-update-ref-errors.sh.
     
         Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
     
7:  5a7df840a8 = 7:  e11a7d8e02 t1507: assert output of rev-parse

Andrei Rybak (7):
  t1005: assert output of ls-files
  t1006: assert error output of cat-file
  t1010: don't create unused files
  t1302: don't create unused file
  t1400: assert output of update-ref
  t1404: don't create unused file
  t1507: assert output of rev-parse

 t/t1005-read-tree-reset.sh    | 15 ++++++++++-----
 t/t1006-cat-file.sh           |  3 ++-
 t/t1010-mktree.sh             |  4 ++--
 t/t1302-repo-version.sh       |  2 +-
 t/t1400-update-ref.sh         |  3 +++
 t/t1404-update-ref-errors.sh  |  1 -
 t/t1507-rev-parse-upstream.sh |  6 ++++--
 7 files changed, 22 insertions(+), 12 deletions(-)

Test 'reset should work' in t1005-read-tree-reset.sh compares two files "expect" and "actual" to assert the expected output of "git ls-files". Several other tests in the same file also create files "expect" and "actual", but don't use them in assertions.

Assert output of "git ls-files" in t1005-read-tree-reset.sh to improve test coverage.

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t1005-read-tree-reset.sh | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

Test "cat-file $arg1 $arg2 error on missing full OID" in t1006-cat-file.sh compares files "expect.err" and "err.actual" to assert the expected error output of "git cat-file". A similar test in the same file named "cat-file $arg1 $arg2 error on missing short OID" also creates these two files, but doesn't use them in assertions.

Assert error output of "git cat-file" in test "cat-file $arg1 $arg2 error on missing short OID" of t1006-cat-file.sh to improve test coverage.

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t1006-cat-file.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Builtin "git mktree" writes the the object name of the tree object built to the standard output. Tests 'mktree refuses to read ls-tree -r output (1)' and 'mktree refuses to read ls-tree -r output (2)' in "t1010-mktree.sh" redirect output of "git mktree" to a file, but don't use its contents in assertions.

Don't redirect output of "git mktree" to file "actual" in tests that assert that an invocation of "git mktree" must fail.

Output of "git mktree" is empty when it refuses to build a tree object. So, alternatively, the test could assert that the output is empty. However, there isn't a good reason for the user to expect the command to be silent in such cases, so we shouldn't enforce it. The user shouldn't use the output of a failing command anyway.

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t1010-mktree.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Test 'gitdir selection on unsupported repo' in t1302-repo-version.sh writes output of a "git config" invocation to file "actual". However, the test doesn't have any assertions for the file. The file was used by this test until commit b9605bc4f2 (config: only read .git/config from configured repos, 2016-09-12), before which "git config" was expected to print the bogus value of "core.repositoryformatversion" to standard output.

Don't redirect output of "git config" to file "actual" in test 'gitdir selection on unsupported repo'.

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t1302-repo-version.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

In t1400-update-ref.sh test 'transaction can create and delete' creates files "expect" and "actual", but doesn't compare them. Similarly, test 'transaction cannot restart ongoing transaction' redirects output of "git update-ref" to file "actual", but doesn't check its contents with any assertions.

Assert output of "git update-ref" in tests to improve test coverage in t1400-update-ref.sh.

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t1400-update-ref.sh | 3 +++
 1 file changed, 3 insertions(+)

Tests in t1507-rev-parse-upstream.sh compare files "expect" and "actual" to assert the output of "git rev-parse", "git show", and "git log". However, two of the tests '@{reflog}-parsing does not look beyond colon' and '@{upstream}-parsing does not look beyond colon' don't inspect the contents of the created files.

Assert output of "git rev-parse" in tests in t1507-rev-parse-upstream.sh to improve test coverage.

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t1507-rev-parse-upstream.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Some tests in file t1404-update-ref-errors.sh create file "unchanged" as the expected side for a test_cmp assertion at the end of the test for output of "git for-each-ref". Test 'no bogus intermediate values during delete' also creates a file named "unchanged" using "git for-each-ref". However, the file isn't used for any assertions in the test. Instead, "git rev-parse" is used to compare the reference with variable $D.

Don't create unused file "unchanged" in test 'no bogus intermediate values during delete' of t1404-update-ref-errors.sh.

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t1404-update-ref-errors.sh | 1 -
 1 file changed, 1 deletion(-)

I messed up when sending v2 (incorrectly used format-patch with --in-reply-to), and v2 didn't get any response. So here's v3 which is just a resend of v2.

Original cover letter is at
  https://lore.kernel.org/git/20230312201520.370234-1-rybak.a.v@gmail.com/T/#m63071efd1e2f876fdcbd0c384130b0ec7859f885

Changes since v1 address Junio's review:

  - just don't redirect output of mktree in patch 3/7
    Cf. https://lore.kernel.org/git/20230312201520.370234-1-rybak.a.v@gmail.com/T/#m0c7b88dcf0af59f65262c00f30dee353170c3b2e
  - rewritten commit message of patch 6/7, where I'd completely misunderstood
    the code of the tests.
    Cf. https://lore.kernel.org/git/20230312201520.370234-1-rybak.a.v@gmail.com/T/#m65f0666a8e94a1d44434dab58ee7002491b13df0

Range diff:

1:  017f1d8173 = 1:  59a868c0b8 t1005: assert output of ls-files
2:  7a75864e00 = 2:  a3514687ad t1006: assert error output of cat-file
3:  e23a16e560 ! 3:  4cb07fa965 t1010: assert empty output of mktree
    @@ Metadata
     Author: Andrei Rybak <rybak.a.v@gmail.com>

      ## Commit message ##
    -    t1010: assert empty output of mktree
    +    t1010: don't create unused files

         Builtin "git mktree" writes the the object name of the tree object built
         to the standard output.  Tests 'mktree refuses to read ls-tree -r output
    @@ Commit message
         "t1010-mktree.sh" redirect output of "git mktree" to a file, but don't
         use its contents in assertions.

    -    Assert that the output of "git mktree" is empty when it refuses to build
    -    a tree object.
    +    Don't redirect output of "git mktree" to file "actual" in tests that
    +    assert that an invocation of "git mktree" must fail.
    +
    +    Output of "git mktree" is empty when it refuses to build a tree object.
    +    So, alternatively, the test could assert that the output is empty.
    +    However, there isn't a good reason for the user to expect the command to
    +    be silent in such cases, so we shouldn't enforce it.  The user shouldn't
    +    use the output of a failing command anyway.

         Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>

    @@ t/t1010-mktree.sh: test_expect_success 'allow missing object with --missing' '

      test_expect_success 'mktree refuses to read ls-tree -r output (1)' '
     -  test_must_fail git mktree <all >actual
    -+  test_must_fail git mktree <all >actual &&
    -+  test_must_be_empty actual
    ++  test_must_fail git mktree <all
      '

      test_expect_success 'mktree refuses to read ls-tree -r output (2)' '
     -  test_must_fail git mktree <all.withsub >actual
    -+  test_must_fail git mktree <all.withsub >actual &&
    -+  test_must_be_empty actual
    ++  test_must_fail git mktree <all.withsub
      '

      test_done
4:  462cfa7025 = 4:  1ed6030a4f t1302: don't create unused file
5:  9fa04e479c = 5:  002942d81c t1400: assert output of update-ref
6:  e79566cc32 ! 6:  2e3446fc2f t1404: don't create unused file
    @@ Commit message

         Some tests in file t1404-update-ref-errors.sh create file "unchanged" as
         the expected side for a test_cmp assertion at the end of the test for
    -    output of "git for-each-ref".  The filename conveys the expectation that
    -    the output won't change between two invocations of "git for-each-ref".
    +    output of "git for-each-ref".  Test 'no bogus intermediate values during
    +    delete' also creates a file named "unchanged" using "git for-each-ref".
    +    However, the file isn't used for any assertions in the test.  Instead,
    +    "git rev-parse" is used to compare the reference with variable $D.

    -    Test 'no bogus intermediate values during delete' also creates a file
    -    named "unchanged".  However, in this test the reference is being
    -    deleted, i.e. it _does change_.  The file itself isn't used for any
    -    assertions in the test.
    -
    -    Don't create the unused and slightly misleading file "unchanged".
    +    Don't create unused file "unchanged" in test 'no bogus intermediate
    +    values during delete' of t1404-update-ref-errors.sh.

         Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>

7:  5a7df840a8 = 7:  e11a7d8e02 t1507: assert output of rev-parse

Andrei Rybak (7):
  t1005: assert output of ls-files
  t1006: assert error output of cat-file
  t1010: don't create unused files
  t1302: don't create unused file
  t1400: assert output of update-ref
  t1404: don't create unused file
  t1507: assert output of rev-parse

 t/t1005-read-tree-reset.sh    | 15 ++++++++++-----
 t/t1006-cat-file.sh           |  3 ++-
 t/t1010-mktree.sh             |  4 ++--
 t/t1302-repo-version.sh       |  2 +-
 t/t1400-update-ref.sh         |  3 +++
 t/t1404-update-ref-errors.sh  |  1 -
 t/t1507-rev-parse-upstream.sh |  6 ++++--
 7 files changed, 22 insertions(+), 12 deletions(-)

Test 'reset should work' in t1005-read-tree-reset.sh compares two files "expect" and "actual" to assert the expected output of "git ls-files". Several other tests in the same file also create files "expect" and "actual", but don't use them in assertions.

Assert output of "git ls-files" in t1005-read-tree-reset.sh to improve test coverage.

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t1005-read-tree-reset.sh | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

Test "cat-file $arg1 $arg2 error on missing full OID" in t1006-cat-file.sh compares files "expect.err" and "err.actual" to assert the expected error output of "git cat-file". A similar test in the same file named "cat-file $arg1 $arg2 error on missing short OID" also creates these two files, but doesn't use them in assertions.

Assert error output of "git cat-file" in test "cat-file $arg1 $arg2 error on missing short OID" of t1006-cat-file.sh to improve test coverage.

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t1006-cat-file.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Builtin "git mktree" writes the the object name of the tree object built to the standard output. Tests 'mktree refuses to read ls-tree -r output (1)' and 'mktree refuses to read ls-tree -r output (2)' in "t1010-mktree.sh" redirect output of "git mktree" to a file, but don't use its contents in assertions.

Don't redirect output of "git mktree" to file "actual" in tests that assert that an invocation of "git mktree" must fail.

Output of "git mktree" is empty when it refuses to build a tree object. So, alternatively, the test could assert that the output is empty. However, there isn't a good reason for the user to expect the command to be silent in such cases, so we shouldn't enforce it. The user shouldn't use the output of a failing command anyway.

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t1010-mktree.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

In t1400-update-ref.sh test 'transaction can create and delete' creates files "expect" and "actual", but doesn't compare them. Similarly, test 'transaction cannot restart ongoing transaction' redirects output of "git update-ref" to file "actual", but doesn't check its contents with any assertions.

Assert output of "git update-ref" in tests to improve test coverage in t1400-update-ref.sh.

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t1400-update-ref.sh | 3 +++
 1 file changed, 3 insertions(+)

Test 'gitdir selection on unsupported repo' in t1302-repo-version.sh writes output of a "git config" invocation to file "actual". However, the test doesn't have any assertions for the file. The file was used by this test until commit b9605bc4f2 (config: only read .git/config from configured repos, 2016-09-12), before which "git config" was expected to print the bogus value of "core.repositoryformatversion" to standard output.

Don't redirect output of "git config" to file "actual" in test 'gitdir selection on unsupported repo'.

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t1302-repo-version.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Some tests in file t1404-update-ref-errors.sh create file "unchanged" as the expected side for a test_cmp assertion at the end of the test for output of "git for-each-ref". Test 'no bogus intermediate values during delete' also creates a file named "unchanged" using "git for-each-ref". However, the file isn't used for any assertions in the test. Instead, "git rev-parse" is used to compare the reference with variable $D.

Don't create unused file "unchanged" in test 'no bogus intermediate values during delete' of t1404-update-ref-errors.sh.

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t1404-update-ref-errors.sh | 1 -
 1 file changed, 1 deletion(-)

Tests in t1507-rev-parse-upstream.sh compare files "expect" and "actual" to assert the output of "git rev-parse", "git show", and "git log". However, two of the tests '@{reflog}-parsing does not look beyond colon' and '@{upstream}-parsing does not look beyond colon' don't inspect the contents of the created files.

Assert output of "git rev-parse" in tests in t1507-rev-parse-upstream.sh to improve test coverage.

Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
---
 t/t1507-rev-parse-upstream.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Andrei Rybak <rybak.a.v@gmail.com> writes:

Everything in this iteration looks good to me.

Let's merge the topic down to 'next'.

Thanks for cleaning up these tests.