FreeBSD & AIX test portability fixes
cover FreeBSD & AIX test portability fixes
This makes the vanilla test suite pass without errors on the FreeBSD system noted in the commit messages.
On AIX things are still horribly broken, but this fixes one more issue that also affects that system.
Ævar Arnfjörð Bjarmason (2):
tests: fix non-portable "${var:-"str"}" construct
tests: fix non-portable iconv invocation
t/t0028-working-tree-encoding.sh | 6 +++++- t/t4013-diff-various.sh | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-)
Signature · 1 line
2.19.0.rc0.228.g281dcd1b4d0
PATCH 1/2 tests: fix non-portable "${var:-"str"}" construct
On both AIX 7200-00-01-1543 and FreeBSD 11.2-RELEASE-p2 the "${var:-"str"}" syntax means something different than what it does under the bash or dash shells.
Both will consider the start of the new unescaped quotes to be a new argument to test_expect_success, resulting in the following error:
error: bug in the test script: 'git diff-tree initial # magic is (not' does not look like a prereq
Fix this by removing the redundant quotes. There's no need for them, and the resulting code works under all the aforementioned shells. This fixes a regression in c2f1d3989 ("t4013: test new output from diff --abbrev --raw", 2017-12-03) first released with Git v2.16.0.
--- t/t4013-diff-various.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
t/t4013-diff-various.sh +1 -1
@@ -140,7 +140,7 @@ do expect="$TEST_DIRECTORY/t4013/diff.$test" actual="$pfx-diff.$test" - test_expect_success "git $cmd # magic is ${magic:-"(not used)"}" ' + test_expect_success "git $cmd # magic is ${magic:-(not used)}" ' { echo "$ git $cmd" case "$magic" in
Signature · 1 line
2.19.0.rc0.228.g281dcd1b4d0
Re: [PATCH 1/2] tests: fix non-portable "${var:-"str"}" construct
Quoted 4 lines · Ævar Arnfjörð Bjarmason
On both AIX 7200-00-01-1543 and FreeBSD 11.2-RELEASE-p2 the "${var:-"str"}" syntax means something different than what it does under the bash or dash shells.
Quoted 8 lines
Both will consider the start of the new unescaped quotes to be a new argument to
test_expect_success, resulting in the following error:error: bug in the test script: 'git diff-tree initial # magic is (not' does not look like a prereq
Fix this by removing the redundant quotes. There's no need for them, and the resulting code works under all the aforementioned shells.
Yup, there is no need for that inner dq pair in this particular case.
I was more worried about scripted Porcelains, like
: "${GIT_OBJECT_DIRECTORY="$(git rev-parse --git-path objects)"}"which I think can safely lose the outer dq pair. In t/ directory, there is this one in test-lib-functions.sh which I do not offhand know how these problematic shells would handle.
echo "#!${2-"$SHELL_PATH"}" &&Other than the presence of these two that are not covered by this patch, the patch itself looks good. Thanks.
Quoted 21 lines
fixes a regression in
c2f1d3989("t4013: test new output from diff --abbrev --raw", 2017-12-03) first released with Git v2.16.0.Signed-off-by Ævar Arnfjörð Bjarmason <[email protected]>--- t/t4013-diff-various.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)t/t4013-diff-various.sh +1
-1@@ -140,7 +140,7 @@ do expect="$TEST_DIRECTORY/t4013/diff.$test" actual="$pfx-diff.$test" - test_expect_success "git $cmd # magic is ${magic:-"(not used)"}" ' + test_expect_success "git $cmd # magic is ${magic:-(not used)}" ' { echo "$ git $cmd" case "$magic" in
Re: [PATCH 1/2] tests: fix non-portable "${var:-"str"}" construct
In reply to Ævar Arnfjörð BjarmasonQuoted 4 lines · Ævar Arnfjörð Bjarmason
Fix this by removing the redundant quotes. There's no need for them, and the resulting code works under all the aforementioned shells. This fixes a regression in
c2f1d3989("t4013: test new output from diff --abbrev --raw", 2017-12-03) first released with Git v2.16.0.
Thanks.
PATCH 2/2 tests: fix non-portable iconv invocation
In reply to Ævar Arnfjörð BjarmasonThe iconv that comes with a FreeBSD 11.2-RELEASE-p2 box I have access to doesn't support the SHIFT-JIS encoding. Guard a test added in e92d62253 ("convert: add round trip check based on 'core.checkRoundtripEncoding'", 2018-04-15) first released with Git v2.18.0 with a prerequisite that checks for its availability.
The iconv command is in POSIX, and we have numerous tests unconditionally relying on its ability to convert ASCII, UTF-8 and UTF-16, but unconditionally relying on the presence of more obscure encodings isn't portable.
--- t/t0028-working-tree-encoding.sh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
t/t0028-working-tree-encoding.sh +5 -1
@@ -203,7 +203,11 @@ test_expect_success 'error if encoding garbage is already in Git' ' test_i18ngrep "error: BOM is required" err.out ' -test_expect_success 'check roundtrip encoding' ' +test_lazy_prereq ICONV_SHIFT_JIS ' + iconv -f UTF-8 -t SHIFT-JIS </dev/null 2>/dev/null +' + +test_expect_success ICONV_SHIFT_JIS 'check roundtrip encoding' ' test_when_finished "rm -f roundtrip.shift roundtrip.utf16" && test_when_finished "git reset --hard HEAD" &&
Signature · 1 line
2.19.0.rc0.228.g281dcd1b4d0
Re: [PATCH 2/2] tests: fix non-portable iconv invocation
Hi,
Quoted 11 lines · Ævar Arnfjörð Bjarmason
The iconv that comes with a FreeBSD 11.2-RELEASE-p2 box I have access to doesn't support the SHIFT-JIS encoding. Guard a test added in
e92d62253("convert: add round trip check based on 'core.checkRoundtripEncoding'", 2018-04-15) first released with Git v2.18.0 with a prerequisite that checks for its availability.The iconv command is in POSIX, and we have numerous tests unconditionally relying on its ability to convert ASCII, UTF-8 and UTF-16, but unconditionally relying on the presence of more obscure encodings isn't portable.
Quoted 4 lines
Signed-off-by Ævar Arnfjörð Bjarmason <[email protected]>--- t/t0028-working-tree-encoding.sh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
One nit:
[...]
t/t0028-working-tree-encoding.sh test_expect_success 'error if encoding garbage is already in Git' ' · line 207
Quoted 11 lines
t/t0028-working-tree-encoding.sh +2
-1@@ -203,7 +203,11 @@ test_expect_success 'error if encoding garbage is already in Git' ' test_i18ngrep "error: BOM is required" err.out ' -test_expect_success 'check roundtrip encoding' ' +test_lazy_prereq ICONV_SHIFT_JIS ' + iconv -f UTF-8 -t SHIFT-JIS </dev/null 2>/dev/null
Is this 2>/dev/null needed? Leaving it out should make the test easier to debug.
If I'm reading it correctly, test_run_lazy_prereq_ appears to respect the normal --verbose etc settings, which means that the 2>/dev/null could be left out. A quick check[*] with and without -v seems to bear this out.
t/t0028-working-tree-encoding.sh test_expect_success 'error if encoding garbage is already in Git' ' · line 212
Quoted 5 lines
t/t0028-working-tree-encoding.sh +3
-0+' + +test_expect_success ICONV_SHIFT_JIS 'check roundtrip encoding' ' test_when_finished "rm -f roundtrip.shift roundtrip.utf16" && test_when_finished "git reset --hard HEAD" &&
With that tweak (removal of 2>/dev/null), this is
Thanks.
[*]
diff --git i/t/t0000-basic.sh w/t/t0000-basic.sh
index 850f651e4e..879f63118e 100755
--- i/t/t0000-basic.sh
+++ w/t/t0000-basic.sh
@@ -25,6 +25,10 @@ try_local_x () {
echo "$x"
}
+test_lazy_prereq NOISY_TEST '
+ echo >&2 "PAAARTY!"
+'
+
# This test is an experiment to check whether any Git users are using # Shells that don't support the "local" keyword. "local" is not # POSIX-standard, but it is very widely supported by POSIX-compliant
@@ -35,7 +39,7 @@ try_local_x () {
# fails this test, you can ignore the failure, but please report the # problem to the Git mailing list <[email protected]>, as it might # convince us to continue avoiding the use of "local". -test_expect_success 'verify that the running shell supports "local"' ' +test_expect_success NOISY_TEST 'verify that the running shell supports "local"' ' x="notlocal" && echo "local" >expected1 && try_local_x >actual1 &&