git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v5 08/10] wildmatch test: create & test files on disk in addition to in-memory

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Jan 30, 2018, 21:21 UTC
Message-ID
<20180130212124.2099-9-avarab@gmail.com>
In-Reply-To
<20180130212124.2099-1-avarab@gmail.com>

There has never been any full roundtrip testing of what git-ls-files and other commands that use wildmatch() actually do, rather we've been satisfied with just testing the underlying C function.

Due to git-ls-files and friends having their own codepaths before they call wildmatch() there's sometimes differences in the behavior between the two. Even when we test for those (as with [1]), there was no one place where you can review how these two modes differ.

Now there is. We now attempt to create a file called $haystack and match $needle against it for each pair of $needle and $haystack that we were passing to test-wildmatch.

If we can't create the file we skip the test. This ensures that we can run this on all platforms and not maintain some infinitely growing whitelist of e.g. platforms that don't support certain characters in filenames.

A notable exception to this is Windows, where due to the reasons explained in [2] the shellscript emulation layer might fake the creation of a file such as "*", and "test -e" for it will succeed since it just got created with some character that maps to "*", but git ls-files won't be fooled by this.

Thus we need to skip creating certain filenames entirely on Windows, the list here might be overly aggressive. I don't have access to a Windows system to test this.

As a result of doing these tests we can now see the cases where these two ways of testing wildmatch differ:

 * Creating a file called 'a[]b' and running ls-files 'a[]b' will show
   that file, but wildmatch("a[]b", "a[]b") will not match
 * wildmatch() won't match a file called \ against \, but ls-files
   will.
 * `git --glob-pathspecs ls-files 'foo**'` will match a file
   'foo/bba/arr', but wildmatch won't, however pathmatch will.
   This seems like a bug to me, the two are otherwise equivalent as
   these tests show.

This also reveals the case discussed in [1], since 2.16.0 '' is now an error as far as ls-files is concerned, but wildmatch() itself happily accepts it.

1. 9e4e8a64c2 ("pathspec: die on empty strings as pathspec",
   2017-06-06)
2. nycvar.QRO.7.76.6.1801052133380.1337@wbunaarf-fpuvaqryva.tvgsbejvaqbjf.bet
   (https://public-inbox.org/git/?q=nycvar.QRO.7.76.6.1801052133380.1337%40wbunaarf-fpuvaqryva.tvgsbejvaqbjf.bet)
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t3070-wildmatch.sh | 201 ++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 190 insertions(+), 11 deletions(-)
diff --git a/t/t3070-wildmatch.sh b/t/t3070-wildmatch.sh
index 3e75cb0cbe..bd11e5acb0 100755
--- a/t/t3070-wildmatch.sh
+++ b/t/t3070-wildmatch.sh
@@ -4,6 +4,72 @@ test_description='wildmatch tests'
 
 . ./test-lib.sh
 
+should_create_test_file() {
+	file=$1
+
+	case $file in
+	# `touch .` will succeed but obviously not do what we intend
+	# here.
+	".")
+		return 1
+		;;
+	# We cannot create a file with an empty filename.
+	"")
+		return 1
+		;;
+	# The tests that are testing that e.g. foo//bar is matched by
+	# foo/*/bar can't be tested on filesystems since there's no
+	# way we're getting a double slash.
+	*//*)
+		return 1
+		;;
+	# When testing the difference between foo/bar and foo/bar/ we
+	# can't test the latter.
+	*/)
+		return 1
+		;;
+	# On Windows, \ in paths is silently converted to /, which
+	# would result in the "touch" below working, but the test
+	# itself failing. See 6fd1106aa4 ("t3700: Skip a test with
+	# backslashes in pathspec", 2009-03-13) for prior art and
+	# details.
+	*\\*)
+		if ! test_have_prereq BSLASHPSPEC
+		then
+			return 1
+		fi
+		# NOTE: The ;;& bash extension is not portable, so
+		# this test needs to be at the end of the pattern
+		# list.
+		#
+		# If we want to add more conditional returns we either
+		# need a new case statement, or turn this whole thing
+		# into a series of "if" tests.
+		;;
+	esac
+
+
+	# On Windows proper (i.e. not Cygwin) many file names which
+	# under Cygwin would be emulated don't work.
+	if test_have_prereq MINGW
+	then
+		case $file in
+		" ")
+			# Files called " " are forbidden on Windows
+			return 1
+			;;
+		*\<*|*\>*|*:*|*\"*|*\|*|*\?*|*\**)
+			# Files with various special characters aren't
+			# allowed on Windows. Sourced from
+			# https://stackoverflow.com/a/31976060
+			return 1
+			;;
+		esac
+	fi
+
+	return 0
+}
+
 match_with_function() {
 	text=$1
 	pattern=$2
@@ -26,25 +92,133 @@ match_with_function() {
 
 }
 
+match_with_ls_files() {
+	text=$1
+	pattern=$2
+	match_expect=$3
+	match_function=$4
+	ls_files_args=$5
+
+	match_stdout_stderr_cmp="
+		tr -d '\0' <actual.raw >actual &&
+		>expect.err &&
+		test_cmp expect.err actual.err &&
+		test_cmp expect actual"
+
+	if test "$match_expect" = 'E'
+	then
+		if test -e .git/created_test_file
+		then
+			test_expect_success "$match_function (via ls-files): match dies on '$pattern' '$text'" "
+				printf '%s' '$text' >expect &&
+				test_must_fail git$ls_files_args ls-files -z -- '$pattern'
+			"
+		else
+			test_expect_failure "$match_function (via ls-files): match skip '$pattern' '$text'" 'false'
+		fi
+	elif test "$match_expect" = 1
+	then
+		if test -e .git/created_test_file
+		then
+			test_expect_success "$match_function (via ls-files): match '$pattern' '$text'" "
+				printf '%s' '$text' >expect &&
+				git$ls_files_args ls-files -z -- '$pattern' >actual.raw 2>actual.err &&
+				$match_stdout_stderr_cmp
+			"
+		else
+			test_expect_failure "$match_function (via ls-files): match skip '$pattern' '$text'" 'false'
+		fi
+	elif test "$match_expect" = 0
+	then
+		if test -e .git/created_test_file
+		then
+			test_expect_success "$match_function (via ls-files): no match '$pattern' '$text'" "
+				>expect &&
+				git$ls_files_args ls-files -z -- '$pattern' >actual.raw 2>actual.err &&
+				$match_stdout_stderr_cmp
+			"
+		else
+			test_expect_failure "$match_function (via ls-files): no match skip '$pattern' '$text'" 'false'
+		fi
+	else
+		test_expect_success "PANIC: Test framework error. Unknown matches value $match_expect" 'false'
+	fi
+}
+
 match() {
-	match_glob=$1
-	match_iglob=$2
-	match_pathmatch=$3
-	match_pathmatchi=$4
-	text=$5
-	pattern=$6
+	if test "$#" = 6
+	then
+		# When test-wildmatch and git ls-files produce the same
+		# result.
+		match_glob=$1
+		match_file_glob=$match_glob
+		match_iglob=$2
+		match_file_iglob=$match_iglob
+		match_pathmatch=$3
+		match_file_pathmatch=$match_pathmatch
+		match_pathmatchi=$4
+		match_file_pathmatchi=$match_pathmatchi
+		text=$5
+		pattern=$6
+	elif test "$#" = 10
+	then
+		match_glob=$1
+		match_iglob=$2
+		match_pathmatch=$3
+		match_pathmatchi=$4
+		match_file_glob=$5
+		match_file_iglob=$6
+		match_file_pathmatch=$7
+		match_file_pathmatchi=$8
+		text=$9
+		pattern=${10}
+	fi
+
+	test_expect_success 'cleanup after previous file test' '
+		if test -e .git/created_test_file
+		then
+			git reset &&
+			git clean -df
+		fi
+	'
+
+	printf '%s' "$text" >.git/expected_test_file
+
+	test_expect_success "setup match file test for $text" '
+		file=$(cat .git/expected_test_file) &&
+		if should_create_test_file "$file"
+		then
+			dirs=${file%/*}
+			if test "$file" != "$dirs"
+			then
+				mkdir -p -- "$dirs" &&
+				touch -- "./$text"
+			else
+				touch -- "./$file"
+			fi &&
+			git add -A &&
+			printf "%s" "$file" >.git/created_test_file
+		elif test -e .git/created_test_file
+		then
+			rm .git/created_test_file
+		fi
+	'
 
 	# $1: Case sensitive glob match: test-wildmatch & ls-files
 	match_with_function "$text" "$pattern" $match_glob "wildmatch"
+	match_with_ls_files "$text" "$pattern" $match_file_glob "wildmatch" " --glob-pathspecs"
 
 	# $2: Case insensitive glob match: test-wildmatch & ls-files
 	match_with_function "$text" "$pattern" $match_iglob "iwildmatch"
+	match_with_ls_files "$text" "$pattern" $match_file_iglob "iwildmatch" " --glob-pathspecs --icase-pathspecs"
 
 	# $3: Case sensitive path match: test-wildmatch & ls-files
 	match_with_function "$text" "$pattern" $match_pathmatch "pathmatch"
+	match_with_ls_files "$text" "$pattern" $match_file_pathmatch "pathmatch" ""
 
 	# $4: Case insensitive path match: test-wildmatch & ls-files
 	match_with_function "$text" "$pattern" $match_pathmatchi "ipathmatch"
+	match_with_ls_files "$text" "$pattern" $match_file_pathmatchi "ipathmatch" " --icase-pathspecs"
 }
 
 # Basic wildmatch features
@@ -113,7 +287,8 @@ match 1 1 1 1 'acrt' 'a[c-c]rt'
 match 0 0 0 0 ']' '[!]-]'
 match 1 1 1 1 'a' '[!]-]'
 match 0 0 0 0 '' '\'
-match 0 0 0 0 '\' '\'
+match 0 0 0 0 \
+      1 1 1 1 '\' '\'
 match 0 0 0 0 'XXX/\' '*/\'
 match 1 1 1 1 'XXX/\' '*/\\'
 match 1 1 1 1 'foo' 'foo'
@@ -127,7 +302,8 @@ match 1 1 1 1 '[ab]' '[[:digit]ab]'
 match 1 1 1 1 '[ab]' '[\[:]ab]'
 match 1 1 1 1 '?a?b' '\??\?b'
 match 1 1 1 1 'abc' '\a\b\c'
-match 0 0 0 0 'foo' ''
+match 0 0 0 0 \
+      E E E E 'foo' ''
 match 1 1 1 1 'foo/bar/baz/to' '**/t[o]'
 
 # Character class tests
@@ -157,8 +333,10 @@ match 1 1 1 1 ']' '[\]]'
 match 0 0 0 0 '\]' '[\]]'
 match 0 0 0 0 '\' '[\]]'
 match 0 0 0 0 'ab' 'a[]b'
-match 0 0 0 0 'a[]b' 'a[]b'
-match 0 0 0 0 'ab[' 'ab['
+match 0 0 0 0 \
+      1 1 1 1 'a[]b' 'a[]b'
+match 0 0 0 0 \
+      1 1 1 1 'ab[' 'ab['
 match 0 0 0 0 'ab' '[!'
 match 0 0 0 0 'ab' '[-'
 match 1 1 1 1 '-' '[-]'
@@ -226,7 +404,8 @@ match 1 1 1 1 foo/bar 'foo/*'
 match 0 0 1 1 foo/bba/arr 'foo/*'
 match 1 1 1 1 foo/bba/arr 'foo/**'
 match 0 0 1 1 foo/bba/arr 'foo*'
-match 0 0 1 1 foo/bba/arr 'foo**'
+match 0 0 1 1 \
+      1 1 1 1 foo/bba/arr 'foo**'
 match 0 0 1 1 foo/bba/arr 'foo/*arr'
 match 0 0 1 1 foo/bba/arr 'foo/**arr'
 match 0 0 0 0 foo/bba/arr 'foo/*z'
-- 
2.15.1.424.g9478a66081
Previous: Ævar Arnfjörð BjarmasonNext: Junio C Hamano
Message 62 of 73 in “increase wildmatch test coverage”
  1. 0/6 increase wildmatch test coverageÆvar Arnfjörð Bjarmason, Dec 23, 2017
  2. 2/6 wildmatch test: use more standard shell styleÆvar Arnfjörð Bjarmason, Dec 23, 2017
  3. 1/6 wildmatch test: indent with tabs, not spacesÆvar Arnfjörð Bjarmason, Dec 23, 2017
  4. 3/6 wildmatch test: use a paranoia pattern from nul_match()Ævar Arnfjörð Bjarmason, Dec 23, 2017
  5. 4/6 wildmatch test: remove dead fnmatch() test codeÆvar Arnfjörð Bjarmason, Dec 23, 2017
  6. 5/6 wildmatch test: perform all tests under all wildmatch() modesÆvar Arnfjörð Bjarmason, Dec 23, 2017
  7. 6/6 wildmatch test: create & test files on disk in addition to in-memoryÆvar Arnfjörð Bjarmason, Dec 23, 2017
  8. Johannes SixtDec 24, 2017
  9. Ævar Arnfjörð BjarmasonDec 24, 2017
  10. Johannes SixtDec 24, 2017
  11. 0/7 increase wildmatch test coverageÆvar Arnfjörð Bjarmason, Dec 25, 2017
  12. 1/7 wildmatch test: indent with tabs, not spacesÆvar Arnfjörð Bjarmason, Dec 25, 2017
  13. 2/7 wildmatch test: use more standard shell styleÆvar Arnfjörð Bjarmason, Dec 25, 2017
  14. 3/7 wildmatch test: don't try to vertically align our outputÆvar Arnfjörð Bjarmason, Dec 25, 2017
  15. 4/7 wildmatch test: use a paranoia pattern from nul_match()Ævar Arnfjörð Bjarmason, Dec 25, 2017
  16. 5/7 wildmatch test: remove dead fnmatch() test codeÆvar Arnfjörð Bjarmason, Dec 25, 2017
  17. 6/7 wildmatch test: perform all tests under all wildmatch() modesÆvar Arnfjörð Bjarmason, Dec 25, 2017
  18. Junio C HamanoDec 28, 2017
  19. 7/7 wildmatch test: create & test files on disk in addition to in-memoryÆvar Arnfjörð Bjarmason, Dec 25, 2017
  20. Johannes SixtDec 25, 2017
  21. Junio C HamanoDec 27, 2017
  22. Adam DinwoodieJan 3, 2018
  23. Ævar Arnfjörð BjarmasonJan 3, 2018
  24. Adam DinwoodieJan 3, 2018
  25. Ævar Arnfjörð BjarmasonJan 3, 2018
  26. Adam DinwoodieJan 4, 2018
  27. 0/7 increase wildmatch test coverageÆvar Arnfjörð Bjarmason, Jan 4, 2018
  28. 1/7 wildmatch test: indent with tabs, not spacesÆvar Arnfjörð Bjarmason, Jan 4, 2018
  29. 2/7 wildmatch test: use more standard shell styleÆvar Arnfjörð Bjarmason, Jan 4, 2018
  30. 3/7 wildmatch test: don't try to vertically align our outputÆvar Arnfjörð Bjarmason, Jan 4, 2018
  31. 4/7 wildmatch test: use a paranoia pattern from nul_match()Ævar Arnfjörð Bjarmason, Jan 4, 2018
  32. 5/7 wildmatch test: remove dead fnmatch() test codeÆvar Arnfjörð Bjarmason, Jan 4, 2018
  33. 6/7 wildmatch test: perform all tests under all wildmatch() modesÆvar Arnfjörð Bjarmason, Jan 4, 2018
  34. 7/7 wildmatch test: create & test files on disk in addition to in-memoryÆvar Arnfjörð Bjarmason, Jan 4, 2018
  35. Johannes SchindelinJan 5, 2018
  36. Ævar Arnfjörð BjarmasonJan 5, 2018
  37. Johannes SchindelinJan 5, 2018
  38. 8/7 wildmatch test: skip file creation tests on Windows properÆvar Arnfjörð Bjarmason, Jan 5, 2018
  39. Junio C HamanoJan 5, 2018
  40. Johannes SchindelinJan 6, 2018
  41. Ævar Arnfjörð BjarmasonJan 6, 2018
  42. Johannes SchindelinJan 6, 2018
  43. Johannes SchindelinJan 8, 2018
  44. Junio C HamanoJan 8, 2018
  45. Duy NguyenJan 7, 2018
  46. Johannes SchindelinJan 8, 2018
  47. Duy NguyenJan 10, 2018
  48. Adam DinwoodieJan 10, 2018
  49. Duy NguyenJan 10, 2018
  50. Johannes SchindelinJan 10, 2018
  51. Duy NguyenJan 11, 2018
  52. 00/10 increase wildmatch test coverageÆvar Arnfjörð Bjarmason, Jan 30, 2018
  53. 01/10 wildmatch test: indent with tabs, not spacesÆvar Arnfjörð Bjarmason, Jan 30, 2018
  54. 02/10 wildmatch test: use more standard shell styleÆvar Arnfjörð Bjarmason, Jan 30, 2018
  55. 03/10 wildmatch test: don't try to vertically align our outputÆvar Arnfjörð Bjarmason, Jan 30, 2018
  56. 04/10 wildmatch test: use a paranoia pattern from nul_match()Ævar Arnfjörð Bjarmason, Jan 30, 2018
  57. 06/10 wildmatch test: use test_must_fail, not ! for test-wildmatchÆvar Arnfjörð Bjarmason, Jan 30, 2018
  58. 05/10 wildmatch test: remove dead fnmatch() test codeÆvar Arnfjörð Bjarmason, Jan 30, 2018
  59. 07/10 wildmatch test: perform all tests under all wildmatch() modesÆvar Arnfjörð Bjarmason, Jan 30, 2018
  60. 09/10 test-lib: add an EXPENSIVE_ON_WINDOWS prerequisiteÆvar Arnfjörð Bjarmason, Jan 30, 2018
  61. 10/10 wildmatch test: mark test as EXPENSIVE_ON_WINDOWSÆvar Arnfjörð Bjarmason, Jan 30, 2018
  62. 08/10 wildmatch test: create & test files on disk in addition to in-memoryÆvar Arnfjörð Bjarmason, Jan 30, 2018
  63. Junio C HamanoDec 28, 2017
  64. Ævar Arnfjörð BjarmasonDec 28, 2017
  65. 0/7 increase wildmatch test coverageÆvar Arnfjörð Bjarmason, Dec 28, 2017
  66. 1/7 wildmatch test: indent with tabs, not spacesÆvar Arnfjörð Bjarmason, Dec 28, 2017
  67. 3/7 wildmatch test: don't try to vertically align our outputÆvar Arnfjörð Bjarmason, Dec 28, 2017
  68. 2/7 wildmatch test: use more standard shell styleÆvar Arnfjörð Bjarmason, Dec 28, 2017
  69. 6/7 wildmatch test: perform all tests under all wildmatch() modesÆvar Arnfjörð Bjarmason, Dec 28, 2017
  70. 4/7 wildmatch test: use a paranoia pattern from nul_match()Ævar Arnfjörð Bjarmason, Dec 28, 2017
  71. 5/7 wildmatch test: remove dead fnmatch() test codeÆvar Arnfjörð Bjarmason, Dec 28, 2017
  72. 7/7 wildmatch test: create & test files on disk in addition to in-memoryÆvar Arnfjörð Bjarmason, Dec 28, 2017
  73. Ævar Arnfjörð BjarmasonDec 29, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.