On Fri, Jan 14, 2022 at 11:47 AM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Elijah Newren <newren@xxxxxxxxx> writes: > > > It's apparently the latter, because there have been no test script > > changes in the relevant tests. > > > >> Somebody with too much time on their hand should go in and check to > >> help, before CI testing on 'seen' becomes useful again. > > > > This "fixes" seen: > > https://lore.kernel.org/git/pull.1192.git.git.1642176433017.gitgitgadget@xxxxxxxxx/ > > > > I briefly looked at a couple leak traces and thought they looked ref > > related, but I don't have time to go hunt down memory leaks right now. > > I figure this thread has reported them, so let's just get "seen" back > > to green. > > If it were "we added a use of known-to-leak command in an otherwise > clean test, without adding a new leak", I would wholeheartedly > support such a change, but if it is the other way around, it may > make sense to leave it broken as an incentive for people who care > about leaks to go in and fix them up. Perhaps. Waiting can make sense up to a point. > If we toggle it off any time leak-checker CI job starts complaining > on a test script, the leak-checker CI job serves no useful purpose, > no? Folks who use PRs for the purpose of getting the cross-platform CI testing before submitting to the list can still get early notification of potential leaks in their own series, due to the remaining tests being marked as leak-free. They can then fix up their series before submitting them to the list. That seems like a useful purpose to me. Further, these CI jobs did notify us of an issue in someone else's patches (we don't yet know whose), and we were able to report it much like any other bug report. That gives people a heads up and allows them to take action on it. (And if they do so, they can remark the test as leak-free.) That also seems like a useful purpose to me. In contrast, if we leave the leak-checker failing and the failing job spreads to next and master, then we'll just end up training everyone to ignore it -- both for their own PRs and in general. To me, that's what making the leak-checker serve no useful purpose would look like.
