Hi Elijah, On Wed, 16 Jan 2019, Elijah Newren wrote: > On Wed, Jan 16, 2019 at 12:31 PM Johannes Schindelin > <Johannes.Schindelin@xxxxxx> wrote: > > > > Point in case: it took me quite a good while to understand the code > > that I fixed in this patch, because the underlying intention was not > > immediately accessible to me. The test failure happens in the *next* > > test case, after all. That was a head scratcher, too: the problem was > > in a test case that *passed*, not in the one that failed. > > Hmm...I had adopted this style based on my own struggles to understand > regression tests. I had gotten pretty frustrated finding a failure in > test #87, and finding out that its setup was spread throughout two dozen > of the preceding 86 tests, with another three dozen changes to the > repository in the preceding 86 tests that happen to be irrelevant. I > like the setup for a test being separately contained. Secondarily, > though, there were a number of tests where I found it hard to see what > they were meaning to test because of a huge mixture of setup and testing > in the same test. Putting the setup in one test and then the actual > thing of interest being tested in a subsequent test made that a whole > lot clearer. > > ...or so I thought. I'm sorry that it to have made it worse for you. I > was trying to make things clearer. Thank you for caring. Truth be told: while it took me like 15 minutes to figure out that the problem was in another test case than the failing one, which is not my personal record (I forget the details, but I am fairly certain that there was a test suite failure that I had to hunt for multiple hours). So it is not all that bad. And yes, having a single setup test case rather than dozens is definitely a good thing. I just miss the expressiveness of object-oriented test frameworks, where you would define data structures with intuitive method names, where I would have immediately thought: oh, so this wanted to merge diverging histories, but, but, but, the first arm does not even have a change... now, where should that have been changed? As I said, your test cases are not the most convoluted ones, and it is so good of you not to add more test cases relying on side effects from previous non-setup test cases (there are quite a few offenders in the test suite, and I am ashamed to admit that even I added some.) > Anyway, with the wording change to the commit message you mentioned > above, feel free to add > > Reviewed-by: Elijah Newren <newren@xxxxxxxxx> Done, thank you so much! Dscho
