On Wednesday, April 03, 2013 13:37:40 Andrei Alexandrescu wrote:
> On 4/2/13 11:03 PM, Jonathan M Davis wrote:
> > I
> > find your focus on trying to keep unit tests to a minimum to be disturbing
> > and likely to lead to poorly tested code.
> 
> Well that's quite the assumption.

If you push for the lines of unit testing code to be kept to a minimum, I don't see how you can possibly expect stuff to be thoroughly tested. There are times that better written tests take up less space, but testing isn't free, and if anything, we need more of it, not less, if we want to make sure that all of Phobos works correctly. And on multiple occasions now, you've balked at what I would consider to be properly thorough unit tests and wanted them to be reduced in size. And since that generally means testing fewer things, I think that it's pretty much a sure thing that it's generally going to lead to poorer testing and increase the risk of code being buggy.

- Jonathan M Davis

On Wednesday, 3 April 2013 at 17:08:57 UTC, Andrei Alexandrescu wrote:
> On 4/3/13 11:55 AM, Peter Alexander wrote:
>> On Wednesday, 3 April 2013 at 02:44:15 UTC, Andrei Alexandrescu wrote:
>>> If we did datetime all over again, I'd give a budget of 2000 lines for
>>> all functionality. I bet the solution would be better.
>>
>> I think you are massively underestimating the complexity and subtleties
>> of dates and time.
>
> May as well. I recall before I approved std.datetime I looked at the implementation sizes of similar functionality in other languages; they were all rather bulky, but std.datetime was at the high end of the range.
>

Boost datetime is 27k.  Just the headers comes to 17k.  A 2k budget for a date time library is unreasonable unless you don't want anyone using D for anything serious involving dates and times.  They are complex and require a lot of code to get right.

Perhaps 34k is too large but 2k is laughable.

On Wednesday, April 03, 2013 19:59:37 Brad Anderson wrote:
> Perhaps 34k is too large but 2k is laughable.

I really should strip out the unit tests and documentation to see what the line count of actual code is, as something like 75% of that is unit tests and documentation, and IIRC, std.datetime provides most of the functionality that Boost does plus some more, though it does some weird, complicated stuff with its header files from what I recall. I'd hate to be the maintainer of Boost's datetime stuff.

- Jonathan M Davis

On Tuesday, April 02, 2013 20:41:23 Walter Bright wrote:
> Currently, the datetime unittest coverage is 95%. Some of the 0 cases suggest low hanging fruit.

I should take another look at those. I thought that I had it at more like 98% (with most or all of the missing lines being due to stuff like catching Exception and asserting 0 in the catch block for making a function nothrow when you know that the code being called will never throw), but that was quite a while ago, and it sounds like it's now missing some stuff. I'm very much in favor of having 100% test coverage on every line that _can_ be tested (there may be rare exceptions to that, but I don't think that std.datetime has any of them).

- Jonathan M Davis

On Wednesday, April 03, 2013 08:53:09 Jacob Carlborg wrote:
> On 2013-04-03 05:03, Jonathan M Davis wrote:
> > I very much doubt that you could do that unless you specifically formatted the code to take up as few lines as possible and didn't count the unit tests or documentation in that line count. Otherwise, you couldn't do anything even close to what std.datetime does in that few lines. Sure, some functionality could be stripped, but you'd end up with something that did a lot less if it were that small. The unit tests and documentation do make it seem like a lot more code than it is, since they take up well over half the file (probably 3/4), but you'd definitely lose functionality with that few lines of code, and you'd end up with something very poor IMHO if those 2000 lines included the documentation and unit tests. You'd either end up with something that was very bare-bones and/or something which was poorly tested, and given how easy it is to screw up some of those date/time calculations, having only a few tests would be a very bad idea.
> 
> Since he wrote "2000 lines for all functionality", I don't think he included unit tests or docs/comments.

That may be, but he does seem to have a habit of including the unit tests in the line count when he doesn't like how many lines of code a new piece of functionality takes up.

> > std.datetime's unit tests do need some refactoring (some of which I've done, but there's still a fair bit of work to do there), which will definitely reduce the number of LOC that they take up, but I don't agree at all with considering the unit tests as part of the LOC of file when discussing keeping LOC to a minimum. And while it's good to avoid repetitive unit tests, I'd much rather have repetitive unit tests which are thorough than short ones which aren't. I find your focus on trying to keep unit tests to a minimum to be disturbing and likely to lead to poorly tested code.
> > 
> > If anything, we need to be more thorough, not less. That doesn't mean that the tests need to look like what std.datetime has (particularly since I purposefully avoided loops and other more complicated constructs when I wrote them originally in order to make them as simple and as far from error-prone as possible), but unit tests need to be thorough, and while we're getting better, Phobos' unit tests frequently aren't thorough enough (particularly in std.range and std.algorithm when it comes to testing a variety of range types). Too many of them just test a few cases to make sure that the most obvious stuff works rather than making sure they test corner cases and whatnot.
> > 
> > - Jonathan M Davis
> 
> I actually prefer to have repetitive unit tests and not using loops to make it clear what they actually do. Here's an example from our code base, in Ruby:
> 
> describe "Swedish" do
> subject { build(:address) { |a| a.country_id = Country::SWEDEN } }
> 
> it { should validate_postal_code(12345) }
> it { should validate_postal_code(85412) }
> 
> it { should_not validate_postal_code(123) }
> it { should_not validate_postal_code(123456) }
> 
> it { should_not validate_postal_code("05412") }
> it { should_not validate_postal_code("fooba") }
> end
> 
> describe "Finnish" do
> subject { build(:address) { |a| a.country_id = Country::FINLAND } }
> 
> it { should validate_postal_code(12345) }
> it { should validate_postal_code(12354) }
> it { should validate_postal_code(41588) }
> 
> it { should validate_postal_code("00123") }
> it { should validate_postal_code("01588") }
> it { should validate_postal_code("00000") }
> 
> it { should_not validate_postal_code(1234) }
> it { should_not validate_postal_code(123456) }
> it { should_not validate_postal_code("fooba") }
> end
> 
> It could be written less repetitive, like this:
> 
> postal_codes = {
> Country::SWEDEN => {
> valid: [12345, 85412],
> invalid: [123, 123456, "05412", "fooba"]
> },
> 
> Country::FINLAND => {
> valid: [12345, 12354, 41588],
> invalid: ["00123", "01588", "00000", 1234, 123456, "fooba"]
> }
> }
> 
> postal_codes.each do |country_id, postal_codes|
> describe c.english_name do
> subject { build(:address) { |a| a.country_id = country_id } }
> 
> postal_codes[:valid].each do |postal_code|
> it { should validate_postal_code(postal_code) }
> end
> 
> postal_codes[:invalid].each do |postal_code|
> it { should_not validate_postal_code(postal_code) }
> end
> end
> end
> 
> But I don't think that looks any better. I think it's much worse.

In general, I agree, because I think that straight-forward tests that avoid loops and the like are far less error-prone, and you need the tests to not be buggy. I don't want to have to test my test code to make sure that it works correctly.

However, I _do_ think that there's something to be said for refactoring the tests later (after the code supposedly fully works) to use loops and other more complicated constructs, because not only can that lead to more compact tests, but it also makes it much easier to make the tests more thorough (without taking many more lines of code). I just think that _starting out_ with the more complicated tests is not necessarily a good idea. Treating unit testing code as if it were the same is normal code doesn't make sense to me, if nothing else, because that would indicate that you're going to have to test your test code, since normal code is complicated enough to require testing. But Andrei and I have argued about this before, and I don't expect us to agree ever on it.

- Jonathan M Davis

On 4/3/2013 11:08 AM, Jonathan M Davis wrote:
> (with most or all of the missing lines being due to stuff like catching
> Exception and asserting 0 in the catch block for making a function nothrow
> when you know that the code being called will never throw)

Why not just mark them as nothrow? Let the compiler statically check it.

On 4/3/2013 11:08 AM, Jonathan M Davis wrote:
> I'm very much in
> favor of having 100% test coverage on every line that _can_ be tested (there
> may be rare exceptions to that, but I don't think that std.datetime has any of
> them).

I'd be shocked if running -cov for the first time *didn't* come up with issues.

On 4/3/13 1:59 PM, Brad Anderson wrote:
> On Wednesday, 3 April 2013 at 17:08:57 UTC, Andrei Alexandrescu wrote:
>> On 4/3/13 11:55 AM, Peter Alexander wrote:
>>> On Wednesday, 3 April 2013 at 02:44:15 UTC, Andrei Alexandrescu wrote:
>>>> If we did datetime all over again, I'd give a budget of 2000 lines for
>>>> all functionality. I bet the solution would be better.
>>>
>>> I think you are massively underestimating the complexity and subtleties
>>> of dates and time.
>>
>> May as well. I recall before I approved std.datetime I looked at the
>> implementation sizes of similar functionality in other languages; they
>> were all rather bulky, but std.datetime was at the high end of the range.
>>
>
> Boost datetime is 27k. Just the headers comes to 17k. A 2k budget for a
> date time library is unreasonable unless you don't want anyone using D
> for anything serious involving dates and times. They are complex and
> require a lot of code to get right.
>
> Perhaps 34k is too large but 2k is laughable.

Agreed. I just pulled that number randomly without having looked at the current line count.

Andrei

On 4/3/2013 10:58 AM, Jonathan M Davis wrote:
> If you push for the lines of unit testing code to be kept to a minimum, I
> don't see how you can possibly expect stuff to be thoroughly tested.

My idea of perfection would be 100% coverage with zero redundancy in the unittests.

In my experience with testing, the technique of "quantity has a quality all its own" style of testing does not produce adequate test coverage - it just simply takes a lot of time to run (which makes it less useful, as one then tends to avoid running them).

On 4/3/2013 9:49 AM, Dmitry Olshansky wrote:
> +1

Stylistic nit:

When writing a one-liner post like this, please do not quote the entire preceding post, especially if it is long. We have great forum software, and the newsreaders as well are great at navigating the threads.

Not to pick on you, but I see this a lot here from many of our participants and finally felt compelled to speak up!

And yes, I know that sometimes people complain that I do the opposite in not quoting enough of the parent.