Author p-ganssle
Recipients FFY00, p-ganssle, serhiy.storchaka
Date 2020-09-08.21:16:42
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1599599803.05.0.898576215689.issue41734@roundup.psfhosted.org>
In-reply-to
Content
I agree with Filipe here — I think the b32encode/b32decode tests were originally written before subtests were available, and this PR has this and other real improvements.

I understand why you'd want to have a policy of "no refactoring for its own sake", but as I argued in the PR 20441 (https://github.com/python/cpython/pull/20441#issuecomment-634773049), it's safer to leave existing tests alone when making changes to the code under test, since there's the possibility that you both introduce an error *and* modify the tests in such a way that doesn't catch the error you introduced. In that case, "refactoring as you go" doesn't really work, and you need a separate PR for improvements like these.

I'm re-opening the ticket for now because I think we should at least discuss this before rejecting it out of hand.


> I am a bit confused, in PR 20441 I first just copied the current b32{encode,decode} tests but was given feedback which resulted in the proposed tests, but now I am being told the opposite, that the tests are better as they currently are.

Sorry about the mixed messages. I think you simply chalk this up to the fact that Serhiy and I apparently disagree about test structure. I reviewed the previous PR and specifically asked for this change, so I think it was a bit rash to close the issue right away (though as someone who has probably prematurely closed his fair share of issues, I should probably not be tossing about stones in the vicinity of my decidedly double-paned domicile).
History
Date User Action Args
2020-09-08 21:16:43p-gansslesetrecipients: + p-ganssle, serhiy.storchaka, FFY00
2020-09-08 21:16:43p-gansslesetmessageid: <1599599803.05.0.898576215689.issue41734@roundup.psfhosted.org>
2020-09-08 21:16:43p-gansslelinkissue41734 messages
2020-09-08 21:16:42p-gansslecreate