On Thu, Oct 10, 2019 at 01:07:32PM +0200, SZEDER Gábor wrote:
> On Wed, Oct 09, 2019 at 02:00:12PM -0700, William Baker via GitGitGadget 
> wrote:
> > diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
> > index 81a375fa0f..87042470ab 100755
> > --- a/t/t7519-status-fsmonitor.sh
> > +++ b/t/t7519-status-fsmonitor.sh
> > @@ -354,4 +354,17 @@ test_expect_success 'discard_index() also discards 
> > fsmonitor info' '
> >     test_cmp expect actual
> >  '
> >  
> > +# This test covers staging/unstaging files that appear at the end of the 
> > index.
> > +# Test files with names beginning with 'z' are used under the assumption 
> > that
> > +# earlier tests do not add/leave index entries that sort below them. 

I just read through Junio's comments on the first version of this
patch, in particular his remarks about this comment.

If this new test case below were run in a dedicated repository, then
this comment wouldn't be necessary, and all my comments below about
that not-really-initial commit would be moot, too.

> > +test_expect_success 'status succeeds after staging/unstaging ' '
> > +   test_commit initial &&
> 
> This is confusing: this is the 29th test case in this script and it
> creates an "initial" commit?!
> 
> The first "setup" test case has already created an initial commit, so
> this should rather be called "second".
> 
> OTOH, none of the later commands in this test case seem to have
> anything to do with this second commit, and indeed the test case works
> even without it (i.e. 'git status' still segfaults without the fix and
> then succeeds with the fix applied), so instead of updating its
> message perhaps it could simply be removed.

> 
> > +   removed=$(test_seq 1 100 | sed "s/^/z/") &&
> > +   touch $removed &&
> > +   git add $removed &&
> > +   test_config core.fsmonitor "$TEST_DIRECTORY/t7519/fsmonitor-env" &&
> > +   FSMONITOR_LIST="$removed" git restore -S $removed &&
> > +   FSMONITOR_LIST="$removed" git status
> > +'
> > +
> >  test_done

Reply via email to