Re: [PATCH 1/5] t4015: refactor --color-moved whitespace test

[Jeff King]

On Thu, Oct 19, 2017 at 01:56:55PM -0700, Stefan Beller wrote:

> On Thu, Oct 19, 2017 at 1:24 PM, Jeff King  wrote:
> 
> >
> > +test_expect_success 'clean up whitespace-test colors' '
> > +   git config --unset color.diff.oldMoved &&
> > +   git config --unset color.diff.newMoved
> > +'
> 
> This could be part of the previous test as
> 
>   test_config color.diff.oldMoved "magenta" &&
>   test_config color.diff.newMoved "cyan" &&
> 
> in the beginning. (That way we also do not pollute the setup,
> but keeping it test local).

It used to be in the previous test as test_config, but part of the setup
refactoring is to keep these common bits across several tests.  Hence we
"git config" in the setup step, and we must "git config --unset" here in
the cleanup. There's only the one test between setup/cleanup right now,
but the point is to add more.

The other alternative (besides repeating ourselves) would be to put all
those common bits into a function and call the function at the top of
each test.

-Peff

Re: [PATCH 1/5] t4015: refactor --color-moved whitespace test

[Stefan Beller]

On Thu, Oct 19, 2017 at 1:24 PM, Jeff King  wrote:

>
> +test_expect_success 'clean up whitespace-test colors' '
> +   git config --unset color.diff.oldMoved &&
> +   git config --unset color.diff.newMoved
> +'

This could be part of the previous test as

  test_config color.diff.oldMoved "magenta" &&
  test_config color.diff.newMoved "cyan" &&

in the beginning. (That way we also do not pollute the setup,
but keeping it test local).

With or without this nit, this is
Reviewed-by: Stefan Beller 

Thanks,
Stefan

[PATCH 1/5] t4015: refactor --color-moved whitespace test

[Jeff King]

In preparation for testing several different whitespace
options, let's split out the setup and cleanup steps of the
whitespace test.

While we're here, let's also switch to using "<<-" to indent
our here-documents properly, and use q_to_tab to more
explicitly mark where we expect whitespace to appear.

Signed-off-by: Jeff King 
---
 t/t4015-diff-whitespace.sh | 49 +++---
 1 file changed, 29 insertions(+), 20 deletions(-)

diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index 87083f728f..164b502405 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -1318,30 +1318,34 @@ test_expect_success 'no effect from --color-moved with 
--word-diff' '
test_cmp expect actual
 '
 
-test_expect_success 'move detection ignoring whitespace ' '
+test_expect_success 'set up whitespace tests' '
git reset --hard &&
-   cat <<\EOF >lines.txt &&
-line 1
-line 2
-line 3
-line 4
-long line 5
-long line 6
-long line 7
-EOF
-   git add lines.txt &&
-   git commit -m "add poetry" &&
-   cat <<\EOF >lines.txt &&
+   # Note that these lines have no leading or trailing whitespace.
+   cat <<-\EOF >lines.txt &&
+   line 1
+   line 2
+   line 3
+   line 4
long line 5
long line 6
long line 7
-line 1
-line 2
-line 3
-line 4
-EOF
-   test_config color.diff.oldMoved "magenta" &&
-   test_config color.diff.newMoved "cyan" &&
+   EOF
+   git add lines.txt &&
+   git commit -m "add poetry" &&
+   git config color.diff.oldMoved "magenta" &&
+   git config color.diff.newMoved "cyan"
+'
+
+test_expect_success 'move detection ignoring whitespace ' '
+   q_to_tab <<-\EOF >lines.txt &&
+   Qlong line 5
+   Qlong line 6
+   Qlong line 7
+   line 1
+   line 2
+   line 3
+   line 4
+   EOF
git diff HEAD --no-renames --color-moved --color |
grep -v "index" |
test_decode_color >actual &&
@@ -1385,6 +1389,11 @@ EOF
test_cmp expected actual
 '
 
+test_expect_success 'clean up whitespace-test colors' '
+   git config --unset color.diff.oldMoved &&
+   git config --unset color.diff.newMoved
+'
+
 test_expect_success '--color-moved block at end of diff output respects 
MIN_ALNUM_COUNT' '
git reset --hard &&
>bar &&
-- 
2.15.0.rc1.560.g5f0609e481