threads / patch / 30273

v4, 4 partst4052: test --stat output with --graph

Subject: [PATCH v4 1/4] t4052: test --stat output with --graph

## tl;dr

17 messages between Apr 18, 2012 and Apr 29, 2012. Diffs are folded; open one to read it.

replies: 16people: 5as markdown or json

Lucian Poston· Apr 18, 2012, 02:29 UTC · lore
From: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>

Add tests which show that the width of the --prefix added by --graph is not taken into consideration when the diff stat output width is calculated.

Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Signed-off-by: Lucian Poston <lucian.poston@gmail.com>
---
 t/t4052-stat-output.sh |   78 +++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 74 insertions(+), 4 deletions(-)
Show changes to t/t4052-stat-output.sh +74 −4
diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh
index 328aa8f..da14984 100755
--- a/t/t4052-stat-output.sh
+++ b/t/t4052-stat-output.sh
@@ -82,11 +82,15 @@ test_expect_success 'preparation for big change tests' '
 cat >expect80 <<'EOF'
  abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 EOF
-
+cat >expect80-graph <<'EOF'
+|  abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+EOF
 cat >expect200 <<'EOF'
  abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 EOF
-
+cat >expect200-graph <<'EOF'
+|  abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+EOF
 while read verb expect cmd args
 do
 	test_expect_success "$cmd $verb COLUMNS (big change)" '
@@ -94,6 +98,14 @@ do
 		grep " | " output >actual &&
 		test_cmp "$expect" actual
 	'
+
+	test "$cmd" != diff || continue
+
+	test_expect_success "$cmd --graph $verb COLUMNS (big change)" '
+		COLUMNS=200 git $cmd $args --graph >output
+		grep " | " output >actual &&
+		test_cmp "$expect-graph" actual
+	'
 done <<\EOF
 ignores expect80 format-patch -1 --stdout
 respects expect200 diff HEAD^ HEAD --stat
@@ -104,7 +116,9 @@ EOF
 cat >expect40 <<'EOF'
  abcd | 1000 ++++++++++++++++++++++++++
 EOF
-
+cat >expect40-graph <<'EOF'
+|  abcd | 1000 ++++++++++++++++++++++++++
+EOF
 while read verb expect cmd args
 do
 	test_expect_success "$cmd $verb not enough COLUMNS (big change)" '
@@ -118,6 +132,20 @@ do
 		grep " | " output >actual &&
 		test_cmp "$expect" actual
 	'
+
+	test "$cmd" != diff || continue
+
+	test_expect_success "$cmd --graph $verb not enough COLUMNS (big change)" '
+		COLUMNS=40 git $cmd $args --graph >output
+		grep " | " output >actual &&
+		test_cmp "$expect-graph" actual
+	'
+
+	test_expect_success "$cmd --graph $verb statGraphWidth config" '
+		git -c diff.statGraphWidth=26 $cmd $args --graph >output
+		grep " | " output >actual &&
+		test_cmp "$expect-graph" actual
+	'
 done <<\EOF
 ignores expect80 format-patch -1 --stdout
 respects expect40 diff HEAD^ HEAD --stat
@@ -129,6 +157,9 @@ EOF
 cat >expect <<'EOF'
  abcd | 1000 ++++++++++++++++++++++++++
 EOF
+cat >expect-graph <<'EOF'
+|  abcd | 1000 ++++++++++++++++++++++++++
+EOF
 while read cmd args
 do
 	test_expect_success "$cmd --stat=width with big change" '
@@ -143,11 +174,25 @@ do
 		test_cmp expect actual
 	'
 
-	test_expect_success "$cmd --stat-graph--width with big change" '
+	test_expect_success "$cmd --stat-graph-width with big change" '
 		git $cmd $args --stat-graph-width=26 >output
 		grep " | " output >actual &&
 		test_cmp expect actual
 	'
+
+	test "$cmd" != diff || continue
+
+	test_expect_success "$cmd --stat-width=width --graph with big change" '
+		git $cmd $args --stat-width=40 --graph >output
+		grep " | " output >actual &&
+		test_cmp expect-graph actual
+	'
+
+	test_expect_success "$cmd --stat-graph-width --graph with big change" '
+		git $cmd $args --stat-graph-width=26 --graph >output
+		grep " | " output >actual &&
+		test_cmp expect-graph actual
+	'
 done <<\EOF
 format-patch -1 --stdout
 diff HEAD^ HEAD --stat
@@ -164,6 +209,9 @@ test_expect_success 'preparation for long filename tests' '
 cat >expect <<'EOF'
  ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 ++++++++++++
 EOF
+cat >expect-graph <<'EOF'
+|  ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 ++++++++++++
+EOF
 while read cmd args
 do
 	test_expect_success "$cmd --stat=width with big change is more balanced" '
@@ -171,6 +219,14 @@ do
 		grep " | " output >actual &&
 		test_cmp expect actual
 	'
+
+	test "$cmd" != diff || continue
+
+	test_expect_success "$cmd --stat=width --graph with big change is balanced" '
+		git $cmd $args --stat-width=60 --graph >output &&
+		grep " | " output >actual &&
+		test_cmp expect-graph actual
+	'
 done <<\EOF
 format-patch -1 --stdout
 diff HEAD^ HEAD --stat
@@ -181,9 +237,15 @@ EOF
 cat >expect80 <<'EOF'
  ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 ++++++++++++++++++++
 EOF
+cat >expect80-graph <<'EOF'
+|  ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 ++++++++++++++++++++
+EOF
 cat >expect200 <<'EOF'
  aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 EOF
+cat >expect200-graph <<'EOF'
+|  aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+EOF
 while read verb expect cmd args
 do
 	test_expect_success "$cmd $verb COLUMNS (long filename)" '
@@ -191,6 +253,14 @@ do
 		grep " | " output >actual &&
 		test_cmp "$expect" actual
 	'
+
+	test "$cmd" != diff || continue
+
+	test_expect_success "$cmd --graph $verb COLUMNS (long filename)" '
+		COLUMNS=200 git $cmd $args --graph >output
+		grep " | " output >actual &&
+		test_cmp "$expect-graph" actual
+	'
 done <<\EOF
 ignores expect80 format-patch -1 --stdout
 respects expect200 diff HEAD^ HEAD --stat

-- 
1.7.3.4
Lucian Poston· Apr 18, 2012, 02:29 UTC · re: Lucian Poston · lore

[PATCH v4 2/4] Adjust stat width calculations to take --graph output into account

The recent change to compute the width of diff --stat did not take into consideration the output from --graph. The consequence is that when both options are used, e.g. in 'log --stat --graph', the lines are too long.

Add output_prefix_length to diff_options. Initialize the value to 0 and only set it when graph.c:diff_output_prefix_callback() is called.

Using output_prefix_length, adjust stat width calculations to take --graph output into account.

t4052: Modify tests to verify that diff stat output is adjusted to compenstate for the history graph tree.

Signed-off-by: Lucian Poston <lucian.poston@gmail.com>
---
 diff.c                 |    2 +-
 diff.h                 |    1 +
 graph.c                |    3 +++
 t/t4052-stat-output.sh |   34 +++++++++++++++++++++++++---------
 4 files changed, 30 insertions(+), 10 deletions(-)
Show changes to 4 files +30 −10

diff.c, diff.h, graph.c, t/t4052-stat-output.sh

diff --git a/diff.c b/diff.c
index 377ec1e..0b61cf7 100644
--- a/diff.c
+++ b/diff.c
@@ -1445,7 +1445,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 	 */
 
 	if (options->stat_width == -1)
-		width = term_columns();
+		width = term_columns() - options->output_prefix_length;
 	else
 		width = options->stat_width ? options->stat_width : 80;
 
diff --git a/diff.h b/diff.h
index cb68743..19d762f 100644
--- a/diff.h
+++ b/diff.h
@@ -150,6 +150,7 @@ struct diff_options {
 	diff_format_fn_t format_callback;
 	void *format_callback_data;
 	diff_prefix_fn_t output_prefix;
+	int output_prefix_length;
 	void *output_prefix_data;
 };
 
diff --git a/graph.c b/graph.c
index 7358416..7e0a099 100644
--- a/graph.c
+++ b/graph.c
@@ -194,8 +194,10 @@ static struct strbuf *diff_output_prefix_callback(struct diff_options *opt, void
 	struct git_graph *graph = data;
 	static struct strbuf msgbuf = STRBUF_INIT;
 
+	assert(opt);
 	assert(graph);
 
+	opt->output_prefix_length = graph->width;
 	strbuf_reset(&msgbuf);
 	graph_padding_line(graph, &msgbuf);
 	return &msgbuf;
@@ -245,6 +247,7 @@ struct git_graph *graph_init(struct rev_info *opt)
 	 */
 	opt->diffopt.output_prefix = diff_output_prefix_callback;
 	opt->diffopt.output_prefix_data = graph;
+	opt->diffopt.output_prefix_length = 0;
 
 	return graph;
 }
diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh
index da14984..d748e5e 100755
--- a/t/t4052-stat-output.sh
+++ b/t/t4052-stat-output.sh
@@ -89,7 +89,7 @@ cat >expect200 <<'EOF'
  abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 EOF
 cat >expect200-graph <<'EOF'
-|  abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+|  abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 EOF
 while read verb expect cmd args
 do
@@ -117,7 +117,7 @@ cat >expect40 <<'EOF'
  abcd | 1000 ++++++++++++++++++++++++++
 EOF
 cat >expect40-graph <<'EOF'
-|  abcd | 1000 ++++++++++++++++++++++++++
+|  abcd | 1000 ++++++++++++++++++++++++
 EOF
 while read verb expect cmd args
 do
@@ -127,12 +127,6 @@ do
 		test_cmp "$expect" actual
 	'
 
-	test_expect_success "$cmd $verb statGraphWidth config" '
-		git -c diff.statGraphWidth=26 $cmd $args >output
-		grep " | " output >actual &&
-		test_cmp "$expect" actual
-	'
-
 	test "$cmd" != diff || continue
 
 	test_expect_success "$cmd --graph $verb not enough COLUMNS (big change)" '
@@ -140,6 +134,28 @@ do
 		grep " | " output >actual &&
 		test_cmp "$expect-graph" actual
 	'
+done <<\EOF
+ignores expect80 format-patch -1 --stdout
+respects expect40 diff HEAD^ HEAD --stat
+respects expect40 show --stat
+respects expect40 log -1 --stat
+EOF
+
+cat >expect40 <<'EOF'
+ abcd | 1000 ++++++++++++++++++++++++++
+EOF
+cat >expect40-graph <<'EOF'
+|  abcd | 1000 ++++++++++++++++++++++++++
+EOF
+while read verb expect cmd args
+do
+	test_expect_success "$cmd $verb statGraphWidth config" '
+		git -c diff.statGraphWidth=26 $cmd $args >output
+		grep " | " output >actual &&
+		test_cmp "$expect" actual
+	'
+
+	test "$cmd" != diff || continue
 
 	test_expect_success "$cmd --graph $verb statGraphWidth config" '
 		git -c diff.statGraphWidth=26 $cmd $args --graph >output
@@ -244,7 +260,7 @@ cat >expect200 <<'EOF'
  aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 EOF
 cat >expect200-graph <<'EOF'
-|  aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+|  aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 EOF
 while read verb expect cmd args
 do

-- 
1.7.3.4
Lucian Poston· Apr 18, 2012, 02:29 UTC · re: Lucian Poston · lore

[PATCH v4 3/4] t4052: Test diff-stat output with minimum columns

When COLUMNS or --stat-width restricts the diff-stat width to near the minimum, 26 columns, the graph_width value becomes negative. Consequently, the graph part of diff-stat is not resized properly.

Signed-off-by: Lucian Poston <lucian.poston@gmail.com>
---
 t/t4052-stat-output.sh |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)
Show changes to t/t4052-stat-output.sh +28 −0
diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh
index d748e5e..6305c99 100755
--- a/t/t4052-stat-output.sh
+++ b/t/t4052-stat-output.sh
@@ -284,6 +284,34 @@ respects expect200 show --stat
 respects expect200 log -1 --stat
 EOF
 
+cat >expect1 <<'EOF'
+ ...aaaaaaa | 1000 ++++++
+EOF
+cat >expect1-graph <<'EOF'
+|  ...aaaaaaa | 1000 ++++++
+EOF
+while read verb expect cmd args
+do
+	test_expect_failure "$cmd $verb prefix greater than COLUMNS (big change)" '
+		COLUMNS=1 git $cmd $args >output
+		grep " | " output >actual &&
+		test_cmp "$expect" actual
+	'
+
+	test "$cmd" != diff || continue
+
+	test_expect_failure "$cmd --graph $verb prefix greater than COLUMNS (big change)" '
+		COLUMNS=1 git $cmd $args --graph >output
+		grep " | " output >actual &&
+		test_cmp "$expect-graph" actual
+	'
+done <<\EOF
+ignores expect80 format-patch -1 --stdout
+respects expect1 diff HEAD^ HEAD --stat
+respects expect1 show --stat
+respects expect1 log -1 --stat
+EOF
+
 cat >expect <<'EOF'
  abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 EOF

-- 
1.7.3.4
Junio C Hamano· Apr 18, 2012, 20:15 UTC · re: Lucian Poston · lore

Re: [PATCH v4 3/4] t4052: Test diff-stat output with minimum columns

Lucian Poston <lucian.poston@gmail.com> writes:
Show 8 quoted lines
> When COLUMNS or --stat-width restricts the diff-stat width to near the
> minimum, 26 columns, the graph_width value becomes negative. Consequently, the
> graph part of diff-stat is not resized properly.
>
> Signed-off-by: Lucian Poston <lucian.poston@gmail.com>
> ---
>  t/t4052-stat-output.sh |   28 ++++++++++++++++++++++++++++
>  1 files changed, 28 insertions(+), 0 deletions(-)
Running t4052 with this patch applied reports

# fixed 2 known breakage(s) # still have 5 known breakage(s) # passed all remaining 81 test(s)

Please mark the ones that already pass as "test_expect_success", leaving the only broken ones as "test_expect_failure". Otherwise the patch to the test that contains the fix would not illustrate what it fixed.

Thanks.
Lucian Poston· Apr 18, 2012, 21:19 UTC · re: Junio C Hamano · lore

Re: [PATCH v4 3/4] t4052: Test diff-stat output with minimum columns

On Wed, Apr 18, 2012 at 13:15, Junio C Hamano <gitster@pobox.com> wrote:
Show 20 quoted lines
> Lucian Poston <lucian.poston@gmail.com> writes:
>
>> When COLUMNS or --stat-width restricts the diff-stat width to near the
>> minimum, 26 columns, the graph_width value becomes negative. Consequently, the
>> graph part of diff-stat is not resized properly.
>>
>> Signed-off-by: Lucian Poston <lucian.poston@gmail.com>
>> ---
>>  t/t4052-stat-output.sh |   28 ++++++++++++++++++++++++++++
>>  1 files changed, 28 insertions(+), 0 deletions(-)
>
> Running t4052 with this patch applied reports
>
> # fixed 2 known breakage(s)
> # still have 5 known breakage(s)
> # passed all remaining 81 test(s)
>
> Please mark the ones that already pass as "test_expect_success", leaving
> the only broken ones as "test_expect_failure".  Otherwise the patch to the
> test that contains the fix would not illustrate what it fixed.
Sorry about that. Fixed in the recently emailed diffs.
Thanks!
Lucian Poston· Apr 18, 2012, 21:09 UTC · re: Lucian Poston · lore

Re: [PATCH v4 3/4] t4052: Test diff-stat output with minimum columns

When COLUMNS or --stat-width restricts the diff-stat width to near the minimum, 26 columns, the graph_width value becomes negative. Consequently, the graph part of diff-stat is not resized properly.

Signed-off-by: Lucian Poston <lucian.poston@gmail.com>
---
 t/t4052-stat-output.sh |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)
Show changes to t/t4052-stat-output.sh +28 −0
diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh
index d748e5e..b8eec8f 100755
--- a/t/t4052-stat-output.sh
+++ b/t/t4052-stat-output.sh
@@ -284,6 +284,34 @@ respects expect200 show --stat
 respects expect200 log -1 --stat
 EOF
 
+cat >expect1 <<'EOF'
+ ...aaaaaaa | 1000 ++++++
+EOF
+cat >expect1-graph <<'EOF'
+|  ...aaaaaaa | 1000 ++++++
+EOF
+while read teststate verb expect cmd args
+do
+	test_expect_$teststate "$cmd $verb prefix greater than COLUMNS (big change)" '
+		COLUMNS=1 git $cmd $args >output
+		grep " | " output >actual &&
+		test_cmp "$expect" actual
+	'
+
+	test "$cmd" != diff || continue
+
+	test_expect_$teststate "$cmd --graph $verb prefix greater than COLUMNS (big change)" '
+		COLUMNS=1 git $cmd $args --graph >output
+		grep " | " output >actual &&
+		test_cmp "$expect-graph" actual
+	'
+done <<\EOF
+success ignores expect80 format-patch -1 --stdout
+failure respects expect1 diff HEAD^ HEAD --stat
+failure respects expect1 show --stat
+failure respects expect1 log -1 --stat
+EOF
+
 cat >expect <<'EOF'
  abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 EOF

-- 
1.7.3.4
Kacper Kornet· Apr 26, 2012, 22:34 UTC · re: Lucian Poston · lore

Re: [PATCH v4 3/4] t4052: Test diff-stat output with minimum columns

On Wed, Apr 18, 2012 at 02:09:33PM -0700, Lucian Poston wrote:
> When COLUMNS or --stat-width restricts the diff-stat width to near the
> minimum, 26 columns, the graph_width value becomes negative. Consequently, the
> graph part of diff-stat is not resized properly.
> Signed-off-by: Lucian Poston <lucian.poston@gmail.com>
> ---
>  t/t4052-stat-output.sh |   28 ++++++++++++++++++++++++++++
>  1 files changed, 28 insertions(+), 0 deletions(-)
Show 7 quoted lines
> diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh
> index d748e5e..b8eec8f 100755
> --- a/t/t4052-stat-output.sh
> +++ b/t/t4052-stat-output.sh
> @@ -284,6 +284,34 @@ respects expect200 show --stat
>  respects expect200 log -1 --stat
>  EOF
Show 31 quoted lines
> +cat >expect1 <<'EOF'
> + ...aaaaaaa | 1000 ++++++
> +EOF
> +cat >expect1-graph <<'EOF'
> +|  ...aaaaaaa | 1000 ++++++
> +EOF
> +while read teststate verb expect cmd args
> +do
> +	test_expect_$teststate "$cmd $verb prefix greater than COLUMNS (big change)" '
> +		COLUMNS=1 git $cmd $args >output
> +		grep " | " output >actual &&
> +		test_cmp "$expect" actual
> +	'
> +
> +	test "$cmd" != diff || continue
> +
> +	test_expect_$teststate "$cmd --graph $verb prefix greater than COLUMNS (big change)" '
> +		COLUMNS=1 git $cmd $args --graph >output
> +		grep " | " output >actual &&
> +		test_cmp "$expect-graph" actual
> +	'
> +done <<\EOF
> +success ignores expect80 format-patch -1 --stdout
> +failure respects expect1 diff HEAD^ HEAD --stat
> +failure respects expect1 show --stat
> +failure respects expect1 log -1 --stat
> +EOF
> +
>  cat >expect <<'EOF'
>   abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  EOF

These tests fail when /bin/sh is mksh. The problem is, mksh set COLUMNS to be at least 12.

-- 
  Kacper Kornet
Zbigniew Jędrzejewski-Szmek· Apr 27, 2012, 09:25 UTC · re: Kacper Kornet · lore

[PATCH] test: skip test with COLUMNS=1 under mksh

mksh does not allow $COLUMNS to be set below 12. Quoting mksh(1) $COLUMNS is "always set, defaults to 80, unless the value as reported by stty(1) is non-zero and sane enough". This applies also to setting it directly for one command:

$ COLUMNS=30 python -c 'import os; print os.environ["COLUMNS"]' 30 $ COLUMNS=20 python -c 'import os; print os.environ["COLUMNS"]' 20 $ COLUMNS=10 python -c 'import os; print os.environ["COLUMNS"]' 98

Let's test if we can set COLUMNS=1, and if not, skip the tests that want to use that. A prereq is added, which is set if $COLUMNS can be set.

In t9002-column.sh, file with expected output was shared between two test cases, but set in the first one. Since the first test case can now be skipped, setting up the expected output is moved outside of the test case.

Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
---

This fixes problems under /bin/sh=mksh in two newly added test files: t4052 and t9002. I think that doing it with a prereq is fairly minimalistic, and the output looks nice.

 t/t4052-stat-output.sh |    6 ++++--
 t/t9002-column.sh      |    5 +++--
 t/test-lib.sh          |    1 +
 3 files changed, 8 insertions(+), 4 deletions(-)
Show changes to 3 files +8 −4

t/t4052-stat-output.sh, t/t9002-column.sh, t/test-lib.sh

diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh
index ced32db..ddd9497 100755
--- a/t/t4052-stat-output.sh
+++ b/t/t4052-stat-output.sh
@@ -292,7 +292,8 @@ cat >expect1-graph <<'EOF'
 EOF
 while read verb expect cmd args
 do
-	test_expect_success "$cmd $verb prefix greater than COLUMNS (big change)" '
+	test_expect_success COLUMNS_CAN_BE_1 \
+		"$cmd $verb prefix greater than COLUMNS (big change)" '
 		COLUMNS=1 git $cmd $args >output
 		grep " | " output >actual &&
 		test_cmp "$expect" actual
@@ -300,7 +301,8 @@ do
 
 	test "$cmd" != diff || continue
 
-	test_expect_success "$cmd --graph $verb prefix greater than COLUMNS (big change)" '
+	test_expect_success COLUMNS_CAN_BE_1 \
+		"$cmd --graph $verb prefix greater than COLUMNS (big change)" '
 		COLUMNS=1 git $cmd $args --graph >output
 		grep " | " output >actual &&
 		test_cmp "$expect-graph" actual
diff --git a/t/t9002-column.sh b/t/t9002-column.sh
index c7d7a65..8998352 100755
--- a/t/t9002-column.sh
+++ b/t/t9002-column.sh
@@ -50,8 +50,7 @@ EOF
 	test_cmp expected actual
 '
 
-test_expect_success 'COLUMNS = 1' '
-	cat >expected <<\EOF &&
+cat >expected <<\EOF
 one
 two
 three
@@ -64,6 +63,8 @@ nine
 ten
 eleven
 EOF
+
+test_expect_success COLUMNS_CAN_BE_1 'COLUMNS = 1' '
 	COLUMNS=1 git column --mode=column <lista >actual &&
 	test_cmp expected actual
 '
diff --git a/t/test-lib.sh b/t/test-lib.sh
index b7d7100..9e2b711 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -615,6 +615,7 @@ case $(uname -s) in
 	;;
 esac
 
+( COLUMNS=1 && test $COLUMNS = 1 ) && test_set_prereq COLUMNS_CAN_BE_1
 test -z "$NO_PERL" && test_set_prereq PERL
 test -z "$NO_PYTHON" && test_set_prereq PYTHON
 test -n "$USE_LIBPCRE" && test_set_prereq LIBPCRE

-- 
1.7.10.433.g48a19
Junio C Hamano· Apr 27, 2012, 16:08 UTC · re: Zbigniew Jędrzejewski-Szmek · lore

Re: [PATCH] test: skip test with COLUMNS=1 under mksh

Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:
Show 11 quoted lines
> mksh does not allow $COLUMNS to be set below 12. Quoting mksh(1)
> $COLUMNS is "always set, defaults to 80, unless the value as reported
> by stty(1) is non-zero and sane enough". This applies also to setting
> it directly for one command:
>
> $ COLUMNS=30 python -c 'import os; print os.environ["COLUMNS"]'
> 30
> $ COLUMNS=20 python -c 'import os; print os.environ["COLUMNS"]'
> 20
> $ COLUMNS=10 python -c 'import os; print os.environ["COLUMNS"]'
> 98
I'd hate to say this, but that shell is broken.
> Let's test if we can set COLUMNS=1, and if not, skip the tests that
> want to use that. A prereq is added, which is set if $COLUMNS can be
> set.

And I think that is a sensible workaround, even though COLUMNS_CAN_BE_1 might not be the best name for it.

> +( COLUMNS=1 && test $COLUMNS = 1 ) && test_set_prereq COLUMNS_CAN_BE_1

This is different from what you wrote in your log message, but I trust that it will fail under the problematic shell?

Will queue; thanks.
Zbigniew Jędrzejewski-Szmek· Apr 28, 2012, 11:09 UTC · re: Junio C Hamano · lore

Re: [PATCH] test: skip test with COLUMNS=1 under mksh

On 04/27/2012 06:08 PM, Junio C Hamano wrote:
Show 15 quoted lines
> Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:
> 
>> mksh does not allow $COLUMNS to be set below 12. Quoting mksh(1)
>> $COLUMNS is "always set, defaults to 80, unless the value as reported
>> by stty(1) is non-zero and sane enough". This applies also to setting
>> it directly for one command:
>>
>> $ COLUMNS=30 python -c 'import os; print os.environ["COLUMNS"]'
>> 30
>> $ COLUMNS=20 python -c 'import os; print os.environ["COLUMNS"]'
>> 20
>> $ COLUMNS=10 python -c 'import os; print os.environ["COLUMNS"]'
>> 98
> 
> I'd hate to say this, but that shell is broken.
Yes.
Show 11 quoted lines
>> Let's test if we can set COLUMNS=1, and if not, skip the tests that
>> want to use that. A prereq is added, which is set if $COLUMNS can be
>> set.
> 
> And I think that is a sensible workaround, even though COLUMNS_CAN_BE_1
> might not be the best name for it.
> 
>> +( COLUMNS=1 && test $COLUMNS = 1 ) && test_set_prereq COLUMNS_CAN_BE_1
> 
> This is different from what you wrote in your log message, but I trust
> that it will fail under the problematic shell?
Yes.
> Will queue; thanks.
Cool.
Zbyszek
Junio C Hamano· Apr 27, 2012, 16:21 UTC · re: Zbigniew Jędrzejewski-Szmek · lore

Re: [PATCH] test: skip test with COLUMNS=1 under mksh

Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:
Show 11 quoted lines
> mksh does not allow $COLUMNS to be set below 12. Quoting mksh(1)
> $COLUMNS is "always set, defaults to 80, unless the value as reported
> by stty(1) is non-zero and sane enough". This applies also to setting
> it directly for one command:
>
> $ COLUMNS=30 python -c 'import os; print os.environ["COLUMNS"]'
> 30
> $ COLUMNS=20 python -c 'import os; print os.environ["COLUMNS"]'
> 20
> $ COLUMNS=10 python -c 'import os; print os.environ["COLUMNS"]'
> 98
I wonder if this is an easier workaround, though.
	env COLUMNS=10 $cmd

In any case, I'd split this patch to three, as these tests belong to two independent topics.

Stefano Lattarini· Apr 27, 2012, 22:35 UTC · re: Junio C Hamano · lore

Re: [PATCH] test: skip test with COLUMNS=1 under mksh

Hi Junio, Zbigniew.
On 04/27/2012 06:21 PM, Junio C Hamano wrote:
Show 18 quoted lines
> Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:
> 
>> mksh does not allow $COLUMNS to be set below 12. Quoting mksh(1)
>> $COLUMNS is "always set, defaults to 80, unless the value as reported
>> by stty(1) is non-zero and sane enough". This applies also to setting
>> it directly for one command:
>>
>> $ COLUMNS=30 python -c 'import os; print os.environ["COLUMNS"]'
>> 30
>> $ COLUMNS=20 python -c 'import os; print os.environ["COLUMNS"]'
>> 20
>> $ COLUMNS=10 python -c 'import os; print os.environ["COLUMNS"]'
>> 98
> 
> I wonder if this is an easier workaround, though.
> 
> 	env COLUMNS=10 $cmd
> 
It works for me:
  $ mksh -c 'COLUMNS=10 env | grep COLUMNS'
  COLUMNS=113
  $ mksh -c 'env COLUMNS=10 env | grep COLUMNS'
  COLUMNS=10
  $ dpkg -l mksh
  ...
  ii  mksh  40.2-2  MirBSD Korn Shell
HTH,
  Stefano
Zbigniew Jędrzejewski-Szmek· Apr 28, 2012, 11:09 UTC · re: Stefano Lattarini · lore

Re: [PATCH] test: skip test with COLUMNS=1 under mksh

On 04/28/2012 12:35 AM, Stefano Lattarini wrote:
Show 36 quoted lines
> Hi Junio, Zbigniew.
> 
> On 04/27/2012 06:21 PM, Junio C Hamano wrote:
>> Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:
>>
>>> mksh does not allow $COLUMNS to be set below 12. Quoting mksh(1)
>>> $COLUMNS is "always set, defaults to 80, unless the value as reported
>>> by stty(1) is non-zero and sane enough". This applies also to setting
>>> it directly for one command:
>>>
>>> $ COLUMNS=30 python -c 'import os; print os.environ["COLUMNS"]'
>>> 30
>>> $ COLUMNS=20 python -c 'import os; print os.environ["COLUMNS"]'
>>> 20
>>> $ COLUMNS=10 python -c 'import os; print os.environ["COLUMNS"]'
>>> 98
>>
>> I wonder if this is an easier workaround, though.
>>
>> 	env COLUMNS=10 $cmd
>>
> It works for me:
> 
>   $ mksh -c 'COLUMNS=10 env | grep COLUMNS'
>   COLUMNS=113
> 
>   $ mksh -c 'env COLUMNS=10 env | grep COLUMNS'
>   COLUMNS=10
> 
>   $ dpkg -l mksh
>   ...
>   ii  mksh  40.2-2  MirBSD Korn Shell
> 
> HTH,
>   Stefano
>

This approach with 'env COLUMNS=1 git ...' looks nicer, and works when invoked directly, but doesn't work when run under the test harness, because bin-wrappers/git uses !/bin/sh, which resets COLUMNS. So this simpler approach doesn't look feasible.

Zbyszek
Junio C Hamano· Apr 29, 2012, 21:28 UTC · re: Zbigniew Jędrzejewski-Szmek · lore

Re: [PATCH] test: skip test with COLUMNS=1 under mksh

Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> writes:
> This approach with 'env COLUMNS=1 git ...' looks nicer, and works when
> invoked directly, but doesn't work when run under the test harness,
> because bin-wrappers/git uses !/bin/sh, which resets COLUMNS. So this
> simpler approach doesn't look feasible.
Sigh... That shell seem to be infinitely broken X-<.
Lucian Poston· Apr 18, 2012, 02:29 UTC · re: Lucian Poston · lore

[PATCH v4 4/4] Prevent graph_width of stat width from falling below min

Update tests in t4052 fixed by this change.
Signed-off-by: Lucian Poston <lucian.poston@gmail.com>
---
 diff.c                 |    6 +++++-
 t/t4052-stat-output.sh |    4 ++--
 2 files changed, 7 insertions(+), 3 deletions(-)
Show changes to 2 files +7 −3

diff.c, t/t4052-stat-output.sh

diff --git a/diff.c b/diff.c
index 0b61cf7..f08b41a 100644
--- a/diff.c
+++ b/diff.c
@@ -1473,8 +1473,12 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 	 * Adjust adjustable widths not to exceed maximum width
 	 */
 	if (name_width + number_width + 6 + graph_width > width) {
-		if (graph_width > width * 3/8 - number_width - 6)
+		if (graph_width > width * 3/8 - number_width - 6) {
 			graph_width = width * 3/8 - number_width - 6;
+			if (graph_width < 6)
+				graph_width = 6;
+		}
+
 		if (options->stat_graph_width &&
 		    graph_width > options->stat_graph_width)
 			graph_width = options->stat_graph_width;
diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh
index 6305c99..ced32db 100755
--- a/t/t4052-stat-output.sh
+++ b/t/t4052-stat-output.sh
@@ -292,7 +292,7 @@ cat >expect1-graph <<'EOF'
 EOF
 while read verb expect cmd args
 do
-	test_expect_failure "$cmd $verb prefix greater than COLUMNS (big change)" '
+	test_expect_success "$cmd $verb prefix greater than COLUMNS (big change)" '
 		COLUMNS=1 git $cmd $args >output
 		grep " | " output >actual &&
 		test_cmp "$expect" actual
@@ -300,7 +300,7 @@ do
 
 	test "$cmd" != diff || continue
 
-	test_expect_failure "$cmd --graph $verb prefix greater than COLUMNS (big change)" '
+	test_expect_success "$cmd --graph $verb prefix greater than COLUMNS (big change)" '
 		COLUMNS=1 git $cmd $args --graph >output
 		grep " | " output >actual &&
 		test_cmp "$expect-graph" actual

-- 
1.7.3.4
Lucian Poston· Apr 18, 2012, 21:12 UTC · re: Lucian Poston · lore

Re: [PATCH v4 4/4] Prevent graph_width of stat width from falling below min

Update tests in t4052 fixed by this change.
Signed-off-by: Lucian Poston <lucian.poston@gmail.com>
---
 diff.c                 |    6 +++++-
 t/t4052-stat-output.sh |   14 +++++++-------
 2 files changed, 12 insertions(+), 8 deletions(-)
Show changes to 2 files +12 −8

diff.c, t/t4052-stat-output.sh

diff --git a/diff.c b/diff.c
index 0b61cf7..f08b41a 100644
--- a/diff.c
+++ b/diff.c
@@ -1473,8 +1473,12 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 	 * Adjust adjustable widths not to exceed maximum width
 	 */
 	if (name_width + number_width + 6 + graph_width > width) {
-		if (graph_width > width * 3/8 - number_width - 6)
+		if (graph_width > width * 3/8 - number_width - 6) {
 			graph_width = width * 3/8 - number_width - 6;
+			if (graph_width < 6)
+				graph_width = 6;
+		}
+
 		if (options->stat_graph_width &&
 		    graph_width > options->stat_graph_width)
 			graph_width = options->stat_graph_width;
diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh
index b8eec8f..ced32db 100755
--- a/t/t4052-stat-output.sh
+++ b/t/t4052-stat-output.sh
@@ -290,9 +290,9 @@ EOF
 cat >expect1-graph <<'EOF'
 |  ...aaaaaaa | 1000 ++++++
 EOF
-while read teststate verb expect cmd args
+while read verb expect cmd args
 do
-	test_expect_$teststate "$cmd $verb prefix greater than COLUMNS (big change)" '
+	test_expect_success "$cmd $verb prefix greater than COLUMNS (big change)" '
 		COLUMNS=1 git $cmd $args >output
 		grep " | " output >actual &&
 		test_cmp "$expect" actual
@@ -300,16 +300,16 @@ do
 
 	test "$cmd" != diff || continue
 
-	test_expect_$teststate "$cmd --graph $verb prefix greater than COLUMNS (big change)" '
+	test_expect_success "$cmd --graph $verb prefix greater than COLUMNS (big change)" '
 		COLUMNS=1 git $cmd $args --graph >output
 		grep " | " output >actual &&
 		test_cmp "$expect-graph" actual
 	'
 done <<\EOF
-success ignores expect80 format-patch -1 --stdout
-failure respects expect1 diff HEAD^ HEAD --stat
-failure respects expect1 show --stat
-failure respects expect1 log -1 --stat
+ignores expect80 format-patch -1 --stdout
+respects expect1 diff HEAD^ HEAD --stat
+respects expect1 show --stat
+respects expect1 log -1 --stat
 EOF
 
 cat >expect <<'EOF'

-- 
1.7.3.4
Zbigniew Jędrzejewski-Szmek· Apr 18, 2012, 08:47 UTC · re: Lucian Poston · lore

Re: [PATCH v4 1/4] t4052: test --stat output with --graph

On 04/18/2012 04:29 AM, Lucian Poston wrote:
> 1/4 Prevent graph_width of stat width from falling below min
> 2/4 t4052: Test diff-stat output with minimum columns
> 3/4 Adjust stat width calculations to take --graph output into account
> 4/4 t4052: test --stat output with --graph
Looks nice.
Zbyszek

← back to recent threads