Re: [PATCH 5/6] fsmonitor: Remove debugging lines from t/t7519-status-fsmonitor.sh

[Johannes Schindelin]

Hi Junio,

On Tue, 19 Dec 2017, Junio C Hamano wrote:

> Alex Vandiver  writes:
> 
> > These were mistakenly left in when the test was introduced, in
> > 1487372d3 ("fsmonitor: store fsmonitor bitmap before splitting index",
> > 2017-11-09)
> >
> > Signed-off-by: Alex Vandiver 
> > ---
> >  t/t7519-status-fsmonitor.sh | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
> > index eb2d13bbc..19b2a0a0f 100755
> > --- a/t/t7519-status-fsmonitor.sh
> > +++ b/t/t7519-status-fsmonitor.sh
> > @@ -307,9 +307,7 @@ test_expect_success 'splitting the index results in the 
> > same state' '
> > dirty_repo &&
> > git update-index --fsmonitor  &&
> > git ls-files -f >expect &&
> > -   test-dump-fsmonitor >&2 && echo &&
> > git update-index --fsmonitor --split-index &&
> > -   test-dump-fsmonitor >&2 && echo &&
> > git ls-files -f >actual &&
> > test_cmp expect actual
> >  '
> 
> Hmph, by default the standard output and standard error streams are
> not shown in the test output, and it would help while debugging test
> failures, so I am not sure if this is a good change.  With the
> previous step [4/6], we can lose the "echo", of course, and I do not
> think we need >&2 redirection there, either.

I think you got it backwards. Sure, this helps debugging, but it hurts
runtime of the entire test suite (which I might have happened to mention a
couple of times takes way too long on Windows, thanks to our choice of
test "framework").

And in the bigger picture, I think that it is very, very easy to insert
those debugging statements when something breaks (we have to do that with
other tests, anyways).

So I am in favor of this patch, and disagree with your assessment, Junio.

Ciao,
Dscho

Re: [PATCH 5/6] fsmonitor: Remove debugging lines from t/t7519-status-fsmonitor.sh

[Junio C Hamano]

Alex Vandiver  writes:

> These were mistakenly left in when the test was introduced, in
> 1487372d3 ("fsmonitor: store fsmonitor bitmap before splitting index",
> 2017-11-09)
>
> Signed-off-by: Alex Vandiver 
> ---
>  t/t7519-status-fsmonitor.sh | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
> index eb2d13bbc..19b2a0a0f 100755
> --- a/t/t7519-status-fsmonitor.sh
> +++ b/t/t7519-status-fsmonitor.sh
> @@ -307,9 +307,7 @@ test_expect_success 'splitting the index results in the 
> same state' '
>   dirty_repo &&
>   git update-index --fsmonitor  &&
>   git ls-files -f >expect &&
> - test-dump-fsmonitor >&2 && echo &&
>   git update-index --fsmonitor --split-index &&
> - test-dump-fsmonitor >&2 && echo &&
>   git ls-files -f >actual &&
>   test_cmp expect actual
>  '

Hmph, by default the standard output and standard error streams are
not shown in the test output, and it would help while debugging test
failures, so I am not sure if this is a good change.  With the
previous step [4/6], we can lose the "echo", of course, and I do not
think we need >&2 redirection there, either.

[PATCH 5/6] fsmonitor: Remove debugging lines from t/t7519-status-fsmonitor.sh

[Alex Vandiver]

These were mistakenly left in when the test was introduced, in
1487372d3 ("fsmonitor: store fsmonitor bitmap before splitting index",
2017-11-09)

Signed-off-by: Alex Vandiver 
---
 t/t7519-status-fsmonitor.sh | 2 --
 1 file changed, 2 deletions(-)

diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
index eb2d13bbc..19b2a0a0f 100755
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -307,9 +307,7 @@ test_expect_success 'splitting the index results in the 
same state' '
dirty_repo &&
git update-index --fsmonitor  &&
git ls-files -f >expect &&
-   test-dump-fsmonitor >&2 && echo &&
git update-index --fsmonitor --split-index &&
-   test-dump-fsmonitor >&2 && echo &&
git ls-files -f >actual &&
test_cmp expect actual
 '
-- 
2.15.1.626.gc4617b774