Re: [PATCH 04/20] abbrev tests: add tests for core.abbrev and --abbrev
From: Junio C Hamano <hidden>
Date: 2018-06-12 18:31:38
Ævar Arnfjörð Bjarmason [off-list ref] writes:
quoted hunk
diff --git a/t/t0014-abbrev.sh b/t/t0014-abbrev.sh new file mode 100755 index 0000000000..1c60f5ff93 --- /dev/null +++ b/t/t0014-abbrev.sh@@ -0,0 +1,118 @@ +#!/bin/sh + +test_description='test core.abbrev and related features' + +. ./test-lib.sh + +tr_d_n() { + tr -d '\n' +}
- I personally would prefer to see the reason for having a helper
function like this to be "make it easier to reason about", rather
"make it shorter to type". tr_d_n feels more about the latter; if
we aimed for the former, this would be called strip_LF or
something.
- In the existing stcipts, it seems that we prefer to spell tr args
with 3-octal, e.g. \012 for LF, \015 for CR.
- Also let's have SP on both sides of (), i.e.
funcname () {
for shell function definitions.
quoted hunk
+cut_tr_d_n_field_n() { + cut -d " " -f $1 | tr_d_n +}
Likewise. Name this not after how it does what it does, but after what it does and why it does so. In other words, if your answer to the question: "what does the caller want?" is "it wants to pick the nth field", then name it "pick_nth_field" or something?
quoted hunk
+test_expect_success 'abbrev empty value handling differs ' ' + test_must_fail git -c core.abbrev= log -1 --pretty=format:%h 2>stderr && + test_i18ngrep "bad numeric config value.*invalid unit" stderr && + + git branch -v --abbrev= | cut_tr_d_n_field_n 3 >branch && + test_byte_count = 40 branch &&
Sounds like a good thing to unify. If anything, --options=value should be stricter than vari.able=value but it is the other way around.
+ git log --abbrev= -1 --pretty=format:%h >log && + test_byte_count = 4 log &&
Makes readers wonder if 4 is about 3 hex plus terminating LF. The reason why this works is because --pretty=format:%h (not --format=%h or --pretty=tformat:%h) uses delimiter semantics and we won't need any LF to show a single record. If we use the helper to measure the length of hexadecimal digits, it may make sense to add a wrapper around test_byte_count that strips LF; that way, the caller can use --format=%h instead and there will be one less thing for the reader to worry about.
quoted hunk
+ git diff --raw --abbrev= HEAD~ >diff && + cut_tr_d_n_field_n 3 <diff >diff.3 && + test_byte_count = 4 diff.3 && + cut_tr_d_n_field_n 4 <diff >diff.4 && + test_byte_count = 4 diff.4 &&
These all depend on the fact that we do not have excessive number of irrelevant objects to force us to abbreviate using more hexdigits than the minimum 4, right? I _think_ that is a reasonable assumption we can depend on, even across the hash function transition. We may want to leave in-code comment, though.
quoted hunk
+ test_must_fail git diff --raw --abbrev= --no-index X Y >diff && + cut_tr_d_n_field_n 3 <diff >diff.3 && + test_byte_count = 4 diff.3 &&