Hi Duy, On Fri, 30 Mar 2018, Duy Nguyen wrote: > On Fri, Mar 30, 2018 at 2:32 PM, Johannes Schindelin > <Johannes.Schindelin@xxxxxx> wrote: > > > > On Thu, 29 Mar 2018, Jeff King wrote: > > > >> On Thu, Mar 29, 2018 at 11:15:33AM -0700, Stefan Beller wrote: > >> > >> > > When calling `git config --unset abc.a` on this file, it leaves this > >> > > (invalid) config behind: > >> > > > >> > > [ > >> > > [xyz] > >> > > key = value > >> > > > >> > > The reason is that we try to search for the beginning of the line (or > >> > > for the end of the preceding section header on the same line) that > >> > > defines abc.a, but as an optimization, we subtract 2 from the offset > >> > > pointing just after the definition before we call > >> > > find_beginning_of_line(). That function, however, *also* performs that > >> > > optimization and promptly fails to find the section header correctly. > >> > > >> > This commit message would be more convincing if we had it in test form. > >> > >> I agree a test might be nice. But I don't find the commit message > >> unconvincing at all. It explains pretty clearly why the bug occurs, and > >> you can verify it by looking at find_beginning_of_line. > >> > >> > [abc]a > >> > > >> > is not written by Git, but would be written from an outside tool or person > >> > and we barely cope with it? > >> > >> Yes, I don't think git would ever write onto the same line. But clearly > >> we should handle anything that's syntactically valid. > > > > I was tempted to add the test case, because it is easy to test it. > > > > But I then decided *not* to add it. Why? Testing is a balance between "can > > do" and "need to do". > > > > Can you imagine that I did *not* run the entire test suite before > > submitting this patch series, because it takes an incredible *90 minutes* > > to run *on a fast Windows machine*? > > What's wrong with firing up a new worktree, run the test suite there > and go back to do something else so you won't waste time just waiting > for test results and submit? Sure there is a mental overhead for > switching tasks, but at 90 minutes, I think it's worth doing. Of course it is worth doing. That's why I often test the end result on Windows (waiting those 90 minutes, but I do not fire up a new worktree, I use my cloud privilege and let Azure/Visual Studio Team Services do the work for me, without slowing down my laptop). What I would love to do, however, would be to test all intermediate patches, too, as that often shows a problem with my frequent reorderings via interactive rebases. And 90 minutes times 9 is... 13 hours and 30 minutes. That's a really long time. I think the best course of action would be to incrementally do away with the shell scripted test framework, in the way you outlined earlier this year. This would *also* buy us a wealth of other benefits, such as better control over the parallelization, resource usage, etc. It would also finally make it easier to introduce something like "smart testing" where code coverage could be computed (this works only for C code, of course, not for the many scripted parts of core Git), and a diff could be inspected to discover which tests *really* need to be run, skipping the tests that would only touch unchanged code. Ciao, Dscho
