| Message ID | 20230312201520.370234-1-rybak.a.v@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Andrei Rybak <rybak.a.v@gmail.com> writes: > Here are the fixes for the issues I've found so far -- I've gone through t0??? > and t1???. 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. > > Andrei Rybak (7): > t1005: assert output of ls-files > t1006: assert error output of cat-file > t1010: assert empty output of mktree > 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 | 6 ++++-- > 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, 24 insertions(+), 12 deletions(-)
On 13/03/2023 23:41, Junio C Hamano wrote: > > 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. Thank you for review. I have limited bandwidth this week, but I'll try to send a v2 incorporating this feedback on the weekend.
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 999d46fafe..ac2614009d 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -836,6 +836,14 @@ test_expect_success () { if test_run_ "$2" then test_ok_ "$1" + if ! echo "$1" | grep -q -E '(setup|preparation)' + then + if ! python3 "$TEST_DIRECTORY/../check_unused_files.py" "$2" + then + BUG "check_unused_files.py found an unused file in test '$1'" + return 1 + fi + fi else test_failure_ "$@" fi
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< ---- ---- >8 ---- Here are the fixes for the issues I've found so far -- I've gone through t0??? and t1???. Andrei Rybak (7): t1005: assert output of ls-files t1006: assert error output of cat-file t1010: assert empty output of mktree 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 | 6 ++++-- 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, 24 insertions(+), 12 deletions(-)