All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Anthony Ramine" <n.oxyde@gmail.com>,
	"Johannes Sixt" <j6t@kdbg.org>,
	"Adam Dinwoodie" <adam@dinwoodie.org>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Kyle J . McKay" <mackyle@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v5 08/10] wildmatch test: create & test files on disk in addition to in-memory
Date: Tue, 30 Jan 2018 21:21:22 +0000	[thread overview]
Message-ID: <20180130212124.2099-9-avarab@gmail.com> (raw)
In-Reply-To: <20180130212124.2099-1-avarab@gmail.com>
In-Reply-To: <20180104192657.28019-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


  parent reply	other threads:[~2018-01-30 21:22 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-23 21:30 [PATCH 0/6] increase wildmatch test coverage Ævar Arnfjörð Bjarmason
2017-12-23 21:30 ` [PATCH 1/6] wildmatch test: indent with tabs, not spaces Ævar Arnfjörð Bjarmason
2017-12-23 21:30 ` [PATCH 2/6] wildmatch test: use more standard shell style Ævar Arnfjörð Bjarmason
2017-12-23 21:30 ` [PATCH 3/6] wildmatch test: use a paranoia pattern from nul_match() Ævar Arnfjörð Bjarmason
2017-12-23 21:30 ` [PATCH 4/6] wildmatch test: remove dead fnmatch() test code Ævar Arnfjörð Bjarmason
2017-12-23 21:30 ` [PATCH 5/6] wildmatch test: perform all tests under all wildmatch() modes Ævar Arnfjörð Bjarmason
2017-12-23 21:30 ` [PATCH 6/6] wildmatch test: create & test files on disk in addition to in-memory Ævar Arnfjörð Bjarmason
2017-12-24  9:24   ` Johannes Sixt
2017-12-24 11:06     ` Ævar Arnfjörð Bjarmason
2017-12-24 11:51       ` Johannes Sixt
2017-12-25  0:28 ` [PATCH v2 0/7] increase wildmatch test coverage Ævar Arnfjörð Bjarmason
2017-12-28 22:48   ` Junio C Hamano
2017-12-28 23:49     ` Ævar Arnfjörð Bjarmason
2017-12-28 23:28   ` [PATCH v3 " Ævar Arnfjörð Bjarmason
2017-12-28 23:28   ` [PATCH v3 1/7] wildmatch test: indent with tabs, not spaces Ævar Arnfjörð Bjarmason
2017-12-28 23:28   ` [PATCH v3 2/7] wildmatch test: use more standard shell style Ævar Arnfjörð Bjarmason
2017-12-28 23:28   ` [PATCH v3 3/7] wildmatch test: don't try to vertically align our output Ævar Arnfjörð Bjarmason
2017-12-28 23:28   ` [PATCH v3 4/7] wildmatch test: use a paranoia pattern from nul_match() Ævar Arnfjörð Bjarmason
2017-12-28 23:28   ` [PATCH v3 5/7] wildmatch test: remove dead fnmatch() test code Ævar Arnfjörð Bjarmason
2017-12-28 23:28   ` [PATCH v3 6/7] wildmatch test: perform all tests under all wildmatch() modes Ævar Arnfjörð Bjarmason
2017-12-28 23:28   ` [PATCH v3 7/7] wildmatch test: create & test files on disk in addition to in-memory Ævar Arnfjörð Bjarmason
2017-12-29  0:16     ` Ævar Arnfjörð Bjarmason
2017-12-25  0:28 ` [PATCH v2 1/7] wildmatch test: indent with tabs, not spaces Ævar Arnfjörð Bjarmason
2017-12-25  0:28 ` [PATCH v2 2/7] wildmatch test: use more standard shell style Ævar Arnfjörð Bjarmason
2017-12-25  0:28 ` [PATCH v2 3/7] wildmatch test: don't try to vertically align our output Ævar Arnfjörð Bjarmason
2017-12-25  0:28 ` [PATCH v2 4/7] wildmatch test: use a paranoia pattern from nul_match() Ævar Arnfjörð Bjarmason
2017-12-25  0:28 ` [PATCH v2 5/7] wildmatch test: remove dead fnmatch() test code Ævar Arnfjörð Bjarmason
2017-12-25  0:28 ` [PATCH v2 6/7] wildmatch test: perform all tests under all wildmatch() modes Ævar Arnfjörð Bjarmason
2017-12-28 20:28   ` Junio C Hamano
2017-12-25  0:28 ` [PATCH v2 7/7] wildmatch test: create & test files on disk in addition to in-memory Ævar Arnfjörð Bjarmason
2017-12-25  9:26   ` Johannes Sixt
2017-12-27 19:07   ` Junio C Hamano
2018-01-03 13:02   ` Adam Dinwoodie
2018-01-03 13:31     ` Ævar Arnfjörð Bjarmason
2018-01-03 14:41       ` Adam Dinwoodie
2018-01-03 19:14         ` Ævar Arnfjörð Bjarmason
2018-01-04 11:50           ` Adam Dinwoodie
2018-01-04 19:26             ` [PATCH v4 0/7] increase wildmatch test coverage Ævar Arnfjörð Bjarmason
2018-01-30 21:21               ` [PATCH v5 00/10] " Ævar Arnfjörð Bjarmason
2018-01-30 21:21               ` [PATCH v5 01/10] wildmatch test: indent with tabs, not spaces Ævar Arnfjörð Bjarmason
2018-01-30 21:21               ` [PATCH v5 02/10] wildmatch test: use more standard shell style Ævar Arnfjörð Bjarmason
2018-01-30 21:21               ` [PATCH v5 03/10] wildmatch test: don't try to vertically align our output Ævar Arnfjörð Bjarmason
2018-01-30 21:21               ` [PATCH v5 04/10] wildmatch test: use a paranoia pattern from nul_match() Ævar Arnfjörð Bjarmason
2018-01-30 21:21               ` [PATCH v5 05/10] wildmatch test: remove dead fnmatch() test code Ævar Arnfjörð Bjarmason
2018-01-30 21:21               ` [PATCH v5 06/10] wildmatch test: use test_must_fail, not ! for test-wildmatch Ævar Arnfjörð Bjarmason
2018-01-30 21:21               ` [PATCH v5 07/10] wildmatch test: perform all tests under all wildmatch() modes Ævar Arnfjörð Bjarmason
2018-01-30 21:21               ` Ævar Arnfjörð Bjarmason [this message]
2018-01-30 21:21               ` [PATCH v5 09/10] test-lib: add an EXPENSIVE_ON_WINDOWS prerequisite Ævar Arnfjörð Bjarmason
2018-01-30 21:21               ` [PATCH v5 10/10] wildmatch test: mark test as EXPENSIVE_ON_WINDOWS Ævar Arnfjörð Bjarmason
2018-01-04 19:26             ` [PATCH v4 1/7] wildmatch test: indent with tabs, not spaces Ævar Arnfjörð Bjarmason
2018-01-04 19:26             ` [PATCH v4 2/7] wildmatch test: use more standard shell style Ævar Arnfjörð Bjarmason
2018-01-04 19:26             ` [PATCH v4 3/7] wildmatch test: don't try to vertically align our output Ævar Arnfjörð Bjarmason
2018-01-04 19:26             ` [PATCH v4 4/7] wildmatch test: use a paranoia pattern from nul_match() Ævar Arnfjörð Bjarmason
2018-01-04 19:26             ` [PATCH v4 5/7] wildmatch test: remove dead fnmatch() test code Ævar Arnfjörð Bjarmason
2018-01-04 19:26             ` [PATCH v4 6/7] wildmatch test: perform all tests under all wildmatch() modes Ævar Arnfjörð Bjarmason
2018-01-04 19:26             ` [PATCH v4 7/7] wildmatch test: create & test files on disk in addition to in-memory Ævar Arnfjörð Bjarmason
2018-01-05 16:41               ` Johannes Schindelin
2018-01-05 19:08                 ` Ævar Arnfjörð Bjarmason
2018-01-05 20:48                   ` Johannes Schindelin
2018-01-05 22:12                     ` [PATCH v4 8/7] wildmatch test: skip file creation tests on Windows proper Ævar Arnfjörð Bjarmason
2018-01-05 23:13                       ` Junio C Hamano
2018-01-06 12:51                         ` Johannes Schindelin
2018-01-06 13:32                           ` Ævar Arnfjörð Bjarmason
2018-01-06 20:46                             ` Johannes Schindelin
2018-01-08 12:46                             ` Johannes Schindelin
2018-01-08 18:49                             ` Junio C Hamano
2018-01-07  2:51                           ` Duy Nguyen
2018-01-08 12:25                             ` Johannes Schindelin
2018-01-10  9:07                               ` Duy Nguyen
2018-01-10 10:38                                 ` Adam Dinwoodie
2018-01-10 10:52                                   ` Duy Nguyen
2018-01-10 20:24                                 ` Johannes Schindelin
2018-01-11  9:25                                   ` Duy Nguyen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180130212124.2099-9-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=adam@dinwoodie.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    --cc=mackyle@gmail.com \
    --cc=n.oxyde@gmail.com \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.