Re: [PATCH v2 01/10] ls-files: add --json to dump the index

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

[-- Attachment #1: Type: text/plain, Size: 3096 bytes --]

Hi Duy,

On Tue, 25 Jun 2019, Johannes Schindelin wrote:

> On Mon, 24 Jun 2019, Nguyễn Thái Ngọc Duy wrote:
>
> > diff --git a/t/t3011-ls-files-json.sh b/t/t3011-ls-files-json.sh
> > new file mode 100755
> > index 0000000000..97bcd814be
> > --- /dev/null
> > +++ b/t/t3011-ls-files-json.sh
> > @@ -0,0 +1,44 @@
> > +#!/bin/sh
> > +
> > +test_description='ls-files dumping json'
> > +
> > +. ./test-lib.sh
> > +
> > +strip_number() {
> > +	for name; do
> > +		echo 's/\("'$name'":\) [0-9]\+/\1 <number>/' >>filter.sed
>
> This does not do what you think it does, in Ubuntu Xenial and on macOS:
>
> https://dev.azure.com/gitgitgadget/git/_build/results?buildId=11408&view=ms.vss-test-web.build-test-results-tab&runId=27736&paneView=debug&resultId=105613
>
> The `\1` is expanded to the ASCII character 001. Therefore your test cases
> fail on almost all platforms.

The `strip_number()`/`strip_string()` approach might look elegant from a
design perspective, but from a readability perspective (and obviously,
when one wants to make those tests more robust and cross-platform), it
would be a lot better to do it more explicitly.

This patch on top of your patch series makes the test run correctly in my
Linux and Windows setup, and much easier to understand:

-- snipsnap --
diff --git a/t/t3011-ls-files-json.sh b/t/t3011-ls-files-json.sh
index 9f4ad4c9cf..8b782c48e0 100755
--- a/t/t3011-ls-files-json.sh
+++ b/t/t3011-ls-files-json.sh
@@ -4,18 +4,6 @@ test_description='ls-files dumping json'

 . ./test-lib.sh

-strip_number() {
-	for name; do
-		echo 's/\("'$name'":\) [0-9]\+/\1 <number>/' >>filter.sed
-	done
-}
-
-strip_string() {
-	for name; do
-		echo 's/\("'$name'":\) ".*"/\1 <string>/' >>filter.sed
-	done
-}
-
 compare_json() {
 	git ls-files --debug-json >json &&
 	sed -f filter.sed json >filtered &&
@@ -35,9 +23,21 @@ test_expect_success 'setup' '
 	echo intent-to-add >ita &&
 	git add -N ita &&

-	strip_number ctime_sec ctime_nsec mtime_sec mtime_nsec &&
-	strip_number device inode uid gid file_offset ext_size last_update &&
-	strip_string oid ident
+	cat >filter.sed <<-\EOF
+	s/\("ctime_sec":\) [0-9]\+/\1 <number>/
+	s/\("ctime_nsec":\) [0-9]\+/\1 <number>/
+	s/\("mtime_sec":\) [0-9]\+/\1 <number>/
+	s/\("mtime_nsec":\) [0-9]\+/\1 <number>/
+	s/\("device":\) [0-9]\+/\1 <number>/
+	s/\("inode":\) [0-9]\+/\1 <number>/
+	s/\("uid":\) [0-9]\+/\1 <number>/
+	s/\("gid":\) [0-9]\+/\1 <number>/
+	s/\("file_offset":\) [0-9]\+/\1 <number>/
+	s/\("ext_size":\) [0-9]\+/\1 <number>/
+	s/\("last_update":\) [0-9]\+/\1 <number>/
+	s/\("oid":\) ".*"/\1 <string>/
+	s/\("ident":\) ".*"/\1 <string>/
+	EOF
 '

 test_expect_success 'ls-files --json, main entries, UNTR and TREE' '
@@ -98,7 +98,9 @@ test_expect_success !SINGLE_CPU 'ls-files --json and multicore extensions' '
 		touch one two three four &&
 		git add . &&
 		cp ../filter.sed . &&
-		strip_number offset &&
+		cat >>filter.sed <<-\EOF &&
+		s/\("offset":\) [0-9]\+/\1 <number>/
+		EOF
 		compare_json eoie
 	)
 '