mafintosh / dns-packet

An abstract-encoding compliant module for encoding / decoding DNS packets
MIT License
201 stars 70 forks source link

Fix udpPayloadSize encoding #51

Closed MartinKolarik closed 5 years ago

MartinKolarik commented 5 years ago

Fixed a typo in the code which caused udpPayloadSize option to be ignored. Tests didn't catch this because the tested value 4096 is also used as a default, so I updated the tests as well to use a non-default value.

pusateri commented 5 years ago

The fact that this wasn’t noticed before is a really bad sign for javascript development. Would Typescript have caught this? I don’t know enough to know the answer but clearly this is a bigger problem than a simple typo.

MartinKolarik commented 5 years ago

Would Typescript have caught this?

It might but it would depend on how good the definitions were. Better tests would catch this as well. There was only one test and that was using the default option value, making it useless.

pusateri commented 5 years ago

But that suggests that anything less than 100% test coverage is insufficient.

mafintosh commented 5 years ago

Awesome, thank you!

mafintosh commented 5 years ago

5.2.1