Alan Conway <[EMAIL PROTECTED]> wrote: ... > I would go further and strongly suggest that no-one should *ever* commit > new functionality or fixes without tests. The exceptions would be a fix > for that makes an existing test failure pass, and pure refactoring work > that isn't supposed to change the behavior of the system. (And even then > you usually find that the existing tests need a bit more coverage as you > go.) > > It's not an onerous requirement: you *always* test your code before you > commit it don't you? Nobody *ever* just says "hey it compiles, and it's > trivial and obviously works" - right?
Well, I confess that I've been known to do that. But I have to admit I've ended up regretting it once or twice, too. Just a minor correction: creating precise and portable tests *can* be quite onerous, in that preparing good tests takes time and energy. But it is well worth the effort. Note however, that sometimes it is very hard to test for a fix. Here's an actual example that I'm contending with, as coreutils maintainer: Here's a patch for a race condition bug from just last week: patch for cp -p to fix race condition with temporary permissions http://article.gmane.org/gmane.comp.gnu.core-utils.bugs/9103 but to detect the problem, you have to observe the permissions of the file in question in the brief interval between cp's open syscall and a subsequent chmod. One way to verify the patch is to strace a specific cp command and examine the order of the syscalls, but using strace is not portable, and its output is not easy to compare or parse. And besides, you can't reliably determine the permissions on a just-created file via syscall output. These days, I rarely check in a bug fix or new feature without at least some minimal test to exercise it, but in this case, it was a challenge[*]. In fact, I haven't yet added the test, but at least, I now know how I'll do it. The test bourne shell script will have to resort to using gdb in batch mode, setting a break point and quitting just after the open syscall that creates the file in question. Then, the bourne shell continues on to verify the permissions on that file. Of course, this means the binary must now have debugging symbols in order for this test to work, and gdb must be available. But it's worth it. [*] If I'd been willing to change cp's copying engine to make it more testable, it would have been a lot easier. E.g., add a testing-only option to make it exit right after the specified open syscall. But infrastructure like that can make the code harder to maintain.
