No, then you need crap to figure out which iteration broke.

Yeah, it's rather annoying

Congrats, you saved 0 lines of code.

I don't agree, and I don't like you blocking me landing code over nits.

I don't consider this a nit. Bugs arising from copy-pasted code is a well understood phenomenon. Number of lines of code is mostly irrelevant here.

I don't think any of you have a very strong case here. As @jasonbcox points out, it's not about line of code - the goal never was to produce a test case as compact as possible. But some level of code repetition in tests can actually be beneficial.

See: https://evgenii.com/blog/code-duplication-in-unit-tests/

It wouldn't be outside of tests, but testing code is somewhat different than regular code. I often make the mistake myself of making my test code too dry.

1. Case one is special and requires it to be outside the for loop anyways.
2. If any of these cases blow up you need additional code to figure out what broke.
3. This isn't a point of correctness, and should be left up to the author of the diff unless there is clearly a readability issue.

I don't believe I *need* a strong case to have *my* code be the way I want it. Rather, the reviewer needs a case as to why they want it changed.

In this case, @jasonbcox thinks it looks cleaner to have a for loop. I disagree. You either need:

for (int i = 1; i < 6; i++) {
    if ( i == 1 ) {
            BOOST_CHECK(CFeeRate(Amount::zero()) == mpool.estimateFee(i), "Failed at block depth " << i);
            continue;
    }
    BOOST_CHECK(mpool.GetMinFee(1) <= mpool.estimateFee(i), "Failed at block depth " << i);
}

Or:

BOOST_CHECK(CFeeRate(Amount::zero()) == mpool.estimateFee(1), "Failed at block depth 1");
for (int i = 2; i < 6; i++) {
    BOOST_CHECK(mpool.GetMinFee(1) <= mpool.estimateFee(i), "Failed at block depth " << i);
}

In one case you have a nasty anti-pattern and unexpected loop boundaries. In the other you break up the logical grouping of what's being tested. In both cases, what I have here looks better, reads better, and is easier to debug given the output from the boost testing framework.

Precisely what clarity is being provided by having a for loop here? What is being improved?

And yes, I've spent more time writing this than "fixing" my diff. The reason is I think that this review process is borked.