Re: [PATCH v1 0/3] fixes for commented out code in tests (was "Re: [PATCH] *: fix typos which duplicate a word")

[Elijah Newren <newren@gmail.com>]

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.