From: Elijah Newren <newren@gmail.com>
Date: Fri, 13 Jan 2023 04:32:39 GMT
Subject: Re: [PATCH v1 0/3] fixes for commented out code in tests (was "Re: [PATCH] *: fix typos which duplicate a word")
Message-ID: <CABPp-BExVRPjO9DsFsqk8NhKcFcS=mxG91VT8HnPHfW0=XyC7A@mail.gmail.com>
In-Reply-To: <CABPp-BFxK7SGs3wsOfozSw_Uvr-ynr+x8ciPV2Rmfx6Nr4si6g@mail.gmail.com>

On Wed, Jan 11, 2023 at 5:54 PM Elijah Newren <newren@gmail.com> wrote:
>
> On Wed, Jan 11, 2023 at 4:05 PM Andrei Rybak <rybak.a.v@gmail.com> wrote:
> >
> > [ I apologize for some people getting this twice -- I messed up when
> >   invoking `git send-email` ]
> >
> > On 2023-01-06T19:25, Eric Sunshine wrote:
> > > Not related to your patch at all, but I notice in this test that the
> > > call to test_when_finished() is commented out:
> > >
> > >     # test_when_finished "stop_daemon_delete_repo test_insensitive" &&
> > >
> > > which makes me wonder if it was commented out while the test was being
> > > debugged but then forgotten, and that the script is now potentially
> > > leaking a running daemon if something in the test fails after the
> > > daemon was started, or if the daemon does not shut down on its own as
> > > it's supposed to do. [cc:+Jeff Hostetler]
> >
> > Here's a patch series that fixes some of the commented out test code.
>
> Patch 2 is obviously correct.  Patches 1 & 3 make sense to me, but it
> would be nice to have someone familiar with fsmonitor look at #3.
>
> As to your notes about other related testcases...
>
> > I skipped changing the following:
> [...]
> > 2. In t6426-merge-skip-unneeded-updates.sh, second part of the test '2c: Modify
> >    b & add c VS rename b->c' is commented out with an explicit "# FIXME:
> >    rename/add conflicts are horribly broken right now;" above the commented out
> >    part.
> >    [ cc Elijah Newren, author of c04ba51739 (t6046: testcases checking whether
> >    updates can be skipped in a merge, 2018-04-19) ]
> [...]
>
> You missed the cc...but I looked up this email since you did cc me for
> patch 2/3.
>
> Yeah, the commented out code was never tested, because the only thing
> I could have tested at the time was incorrect results.  So I just took
> a guess at what the improved testing would look like and apparently
> made 3 small errors in doing so.  I have fixed it locally and can
> submit a patch.

Submitted here:
https://lore.kernel.org/git/pull.1462.git.1673584084761.gitgitgadget@gmail.com/