4 ms·
> because ossl_a2ulabel had no unit tests until a few days ago it's not realistic to enforce unit test coverage % with a project at the scale of OpenSSL, right
by MuffinFlavored 4y ago
> because ossl_a2ulabel had no unit tests until a few days ago
it's not realistic to enforce unit test coverage % with a project at the scale of OpenSSL, right?
- pca006132 4y agoIt should be realistic, line coverage isn't really that hard. The hard thing is that high line coverage alone is usually not enough for numerical stuff...
- jeffbee 4y agoIt is trivial to enforce that new functions have new unit tests and fuzz tests. You are the reviewer of https://github.com/openssl/openssl/pull/9654 https://github.com/openssl/openssl/pull/9654 and you just say "Please add unit tests and fuzz tests for foo and bar" and you don't approve it. I don't know what the deal is with their testing culture but in year 27 of the project they demonstrably haven't learned this lesson. It's nice that they added integration tests (testing given encoded certs) but as the article points out that was insufficient.
- dralley 4y agoIMO, one of the biggest benefits of "modern" systems languages like Rust, D, Zig is how much easier they make writing and running tests compared to C and C++. Yes, you can write tests for those languages, but it's nowhere near as trivial. And that makes a difference.
- fisf 4y agoI am not buying it. Writing unit tests for c/c++ is trivial. There are perfectly fine test frameworks, used by developers every day, integrated in any major IDE or runnable as one-liner from the command line. This is absolutely a cultural problem.
- edgyquant 4y agoIn modern languages you either have built in testing or it’s a single package install away. It is nowhere near this simple with C or C++
- pjmlp 4y agoI was writing unit tests for C in 1996, naturally we still haven't coined the term, so we just called them automatic tests. It was part of our data structures and algorithms project, failure to execute the automatic tests meant no admission to the final exam. We had three sets of tests, those provided initially at the begin of the term, those that we were expected to write ourselves, and a surprise set on the integration week at the end of the semester.
- bluGill 4y agoLast week in a code review I got a "please add unit tests for this code" comment. The person who wrote that comment wasn't aware that this was a refactoring where the functionality was well tested. There is no substitute for reviewers who really understand the code in question. The problem is they are the ones writing the code and so are biased and not able to give a good review.
- masklinn 4y ago> it's not realistic to enforce unit test coverage % with a project at the scale of OpenSSL, right? Why not? You can enforce that all new files should be covered (at the very least line-covered). It requires some setup effort (collecting code coverage and either sending it to a tool which perform the correlation or correlating yourself), but once that's done... it does its thing. Then you can work on increasing coverage for existing files, and ratcheting requirements.
- stefan_ 4y agoNo one is asking for that. But this is code that did one thing: punycode decoding, with millions of well documented test vectors. The code had zero dependencies on anything OpenSSL related. It is a very simple "text in, text out" problem, the most trivial thing to write unit tests for. At the same time, it's code that parses externally provided buffers and has to deal with things like unicode in C - there should be a massive red flashing warning light in every developers head here.
- duped 4y agoIt's realistic to reject a CR for a new parsing function without proof that it works, which usually comes in the form of a unit test
- planede 4y agoNit: a unit test never proves that it works. At best it can prove that it doesn't work. Otherwise, I agree.
- hardware2win 4y agoNit: "never"? Even if you unit test all cases?
- tz18 4y agoYes, never. You are assuming that the implementation of all unit test cases are themselves correct (that they would fail if there was any error in the case they cover). In fact unit tests are often wrong. In that context a unit test can't even prove code incorrect, unless we know that the unit test is correct. IMO to prove that code is correct requires a proof; a unit test can only provide evidence suggestive of correctness.
- planede 4y agoMistakes in proofs are just as probable as mistakes in exhaustive tests. An exhaustive test is just one type of a machine verified proof.
- nicoburns 4y ago> An exhaustive test is just one type of a machine verified proof. Not entirely sure I agree with this. A proof by construction is a very different beast to empirical unit tests that only cover a subset of inputs. The equivalent would be units tests that cover every single possible input.
- asveikau 4y agoIt's worth noting that coverage can be a deceptive metric sometimes. You can have coverage on code that divides - it won't tell you if you ever divide by zero. You can have coverage on code that follows a pointer - it won't tell you if you ever pass a bad pointer.