search

insomniacslk / dhcp

DHCPv6 and DHCPv4 packet library, client and server written in Go
BSD 3-Clause "New" or "Revised" License
703 stars 170 forks source link

Consider moving away from assert libraries #79

Open insomniacslk opened 6 years ago

insomniacslk commented 6 years ago

As discussed in https://github.com/insomniacslk/dhcp/pull/71 , it is worth considering moving away from assertion libraries like the one we use, github.com/stretchr/testify.

get9 commented 6 years ago

I tried to find some of the relevant documentation on this and... I’m not really convinced this is really worth the time. Sure, Equals does some magic runtime comparisons, but I think that’s better than having to write yet more “if foo != bar” lines after every operation.

insomniacslk commented 6 years ago

This is just a question, not a proposal - I am not saying we have to move away from stretchr/testify, just that it's worth doing some research to understand pros and cons. When I'll have researched and studied more on the matter, I'll be able to decide if I'll suggest to move away from it or not

davebarrau commented 5 years ago

Hey @insomniacslk, did you come to any conclusion on this? Thinking about writing some test cases and don't want to use the wrong library (or have them need to be rewritten later on).

I think for unit testing, it doesn't matter if you fail early, as the end game is you get all tests to pass prior to committing, and you should be running tests often enough that a whole bunch should not blow up at the same time.

I also think the level of automatic detail when using the assert/require libs is great, and it keeps the tests concise.

The point that was made about it not being idiomatic Go.. does it really matter for test code? Happy to go with either option though.

insomniacslk commented 5 years ago

Hey @davebarrau, For now I don't have yet evidence of the benefits of moving away from the testify library, so we will probably stick with it for the foreseeable future. And as you suggest, for our use case this seems to be just fine. I am ok to continue using the current assertion library, and keep this open for reconsideration in the future. If we eventually realize that all this was a mistake I'm ok with adjusting all the tests