
On Sun, Dec 09, 2007 at 01:25:48AM -0800, David Benbennick wrote:
On 12/8/07, David Benbennick
wrote: I commented out some code that could never be executed: * Some case-statement cases that could never occur. * >>= for the Identity monad used internally.
Sorry, I forgot to run the "validate" script on the previous patch. Turns out validate requires >>= to be explicitly set, even though it isn't used. New patch bundle is attached.
Sorry, I've had this sitting in my mailbox for some time, but only just got around to looking at it properly. Thanks for writing it! If I export test_all and make a program that runs it then it takes almost exactly 1 minute to test this one module, as compared to 19 minutes to run the whole testsuite in fast mode, so I don't think that running this as part of the fast testsuite is feasible. I am not even convinced that it makes sense to run it as part of the full testsuite - I don't think we can afford to spend an hour for every 60 modules that we want to test. You might claim that it is taking so long because it is testing IntSet thoroughly, but I claim that it is merely testing it a lot (and, I suspect, testing some cases many times). I believe that a much smaller number of carefully chosen unit tests could test the library just as well, and two or three orders of magnitude more quickly. I hope that soonish we will add hpc support to the testsuite, so we can signal a failure if we don't get 100% coverage. So, in conclusion, I don't think that we should apply this patch (or, more generally, use QC tests in the testsuite). Thanks Ian