On Wed, Jan 11, 2023 at 4:05 PM Andrei Rybak <rybak.a.v@xxxxxxxxx> 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.
