Re: [PATCH 2/2] Revert "remote-curl: fall back to basic auth if Negotiate fails"

On Wed, May 19, 2021 at 09:58:50AM -0400, Derrick Stolee wrote:

quoted

(Note that this isn't a pure revert; the previous commit added a test
showing the regression, so we can now flip it to expect_success).

Keeping the test is excellent, because it gives us a way to confirm
that a second attempt at a fix is at least as good as the first.

The only thing that could improve this situation is to add a test
that checks the bug that the previous version introduced, so that
the next round doesn't repeat the mistake. That can be deferred
because it is more important that we get this fix in time for the
next release candidate.

Re-reading what I wrote, I think "the previous commit" may be ambiguous.
The original commit which introduced the bug (and which we're reverting
here) didn't include a test at all. In patch 1/2 of this series (what
I'm calling "the previous commit"), I provided a test which shows the
regression. And now this revert shows that we fixed it (by flipping from
expect_failure to expect_success).

So I think I've already done what you're asking (if I understand it
correctly).

It probably would be a little easier to follow by reverting first, and
then adding in the expect_success test on top to future-proof us. But by
doing it in the other order, it was easy to see the test demonstrate the
behavior before and after the revert.

-Peff