View mode: basic / threaded / horizontal-split · Log in · Help
March 18, 2013
Re: Raising the bar on Phobos unittest coverage
On Monday, 18 March 2013 at 20:30:32 UTC, Jonathan M Davis wrote:
> On Monday, March 18, 2013 12:09:47 Walter Bright wrote:
>> On 3/18/2013 11:12 AM, Jonathan M Davis wrote:
>> > On Monday, March 18, 2013 17:34:12 Martin Nowak wrote:
>> >> Can someone look into std.file's unittests. They use 60% 
>> >> (32s/50s) of
>> >> the unittest RUNtime on my machine.
>> > 
>> > My first guess would be that you're running a different OS 
>> > from Walter and
>> > that OS-specific code counts as not being run when you run 
>> > it on a
>> > different OS.
>> Code that is statically compiled out is not counted as 
>> executable code by
>> the coverage analyzer.
>
> Then it was a bad guess. Good to know (and better behavior that 
> way really).

Is it though? It basically means that if you don't instantiate a
template, it statically doesn't get compiled, so it is not
counted:

main.d:
//--------
        |void foo(T)()
        |{
        |   int i = 0;
        |   ++i;
        |   ++i;
        |   ++i;
        |   ++i;
        |   ++i;
        |   ++i;
        |   ++i;
        |   ++i;
        |   ++i;
        |   ++i;
        |   ++i;
        |   ++i;
        |   ++i;
        |   ++i;
        |   ++i;
        |   ++i;
        |   ++i;
        |   ++i;
        |   ++i;
        |   ++i;
        |}
        |
        |void main()
        |{
       1|    int i;
        |}
main.d is 100% covered
//--------

main.d 100% covered my ass.

I also call BS on std.algorithm's 95% coverage. I looked at the
.lst. It is *roughly* covered, but not 95% covered.

We need to be able to differentiate code that is stripped out due
to "version" blocks (which legitimately doesn't count for the
current build), and code that simply didn't get compiled because
it WASN'T COVERED.
March 18, 2013
Re: Raising the bar on Phobos unittest coverage
On 3/18/2013 2:09 PM, Jonas Drewsen wrote:
> Please note that std.net.curl needs to have PHOBOS_TEST_ALLOW_NET env variable
> defined or else it will early out on most unittests. I guess that will increase
> the coverage from 2 to something more sane.

Perhaps that should be a -version=TEST_ALLOW_NET setting rather than an 
environment variable.
March 18, 2013
Re: Raising the bar on Phobos unittest coverage
On 03/18/2013 09:30 PM, Jonathan M Davis wrote:
>>> Can someone look into std.file's unittests. They use 60% (32s/50s) of
>>> > >>the unittest RUNtime on my machine.
>> > >
>> > >My first guess would be that you're running a different OS from Walter and
>> > >that OS-specific code counts as not being run when you run it on a
>> > >different OS.
>>Code that is statically compiled out is not counted as executable code by
>>the coverage analyzer.

Sorry for the misunderstanding, but I really meant runtime as in time.
It's great that we're striving for more unittest coverage but we should 
keep an eye on the time it takes to run them.
So I ran the executables with /bin/time and std.file accounts for 60% of 
the total runtime (not including compilation). Even though file IO is 
notoriously slow this seems excessive.

https://github.com/D-Programming-Language/phobos/pull/653
March 18, 2013
Re: Raising the bar on Phobos unittest coverage
On Monday, 18 March 2013 at 22:06:55 UTC, Walter Bright wrote:
> On 3/18/2013 2:09 PM, Jonas Drewsen wrote:
>> Please note that std.net.curl needs to have 
>> PHOBOS_TEST_ALLOW_NET env variable
>> defined or else it will early out on most unittests. I guess 
>> that will increase
>> the coverage from 2 to something more sane.
>
> Perhaps that should be a -version=TEST_ALLOW_NET setting rather 
> than an environment variable.

Maybe.. I did it like that so I wouldn't have to recompile 
everytime I needed to change that flag.
March 18, 2013
Re: Raising the bar on Phobos unittest coverage
On Monday, March 18, 2013 22:13:36 monarch_dodra wrote:
> On Monday, 18 March 2013 at 20:30:32 UTC, Jonathan M Davis wrote:
> > On Monday, March 18, 2013 12:09:47 Walter Bright wrote:
> >> On 3/18/2013 11:12 AM, Jonathan M Davis wrote:
> >> > On Monday, March 18, 2013 17:34:12 Martin Nowak wrote:
> >> >> Can someone look into std.file's unittests. They use 60%
> >> >> (32s/50s) of
> >> >> the unittest RUNtime on my machine.
> >> > 
> >> > My first guess would be that you're running a different OS
> >> > from Walter and
> >> > that OS-specific code counts as not being run when you run
> >> > it on a
> >> > different OS.
> >> 
> >> Code that is statically compiled out is not counted as
> >> executable code by
> >> the coverage analyzer.
> > 
> > Then it was a bad guess. Good to know (and better behavior that
> > way really).
> 
> Is it though? It basically means that if you don't instantiate a
> template, it statically doesn't get compiled, so it is not
> counted:
> 
> main.d:
> //--------
> 
> |void foo(T)()
> |{
> |
> | int i = 0;
> | ++i;
> | ++i;
> | ++i;
> | ++i;
> | ++i;
> | ++i;
> | ++i;
> | ++i;
> | ++i;
> | ++i;
> | ++i;
> | ++i;
> | ++i;
> | ++i;
> | ++i;
> | ++i;
> | ++i;
> | ++i;
> | ++i;
> | ++i;
> |
> |}
> |
> |void main()
> |{
> 
> 1| int i;
> 
> |}
> 
> main.d is 100% covered
> //--------
> 
> main.d 100% covered my ass.
> 
> I also call BS on std.algorithm's 95% coverage. I looked at the
> .lst. It is *roughly* covered, but not 95% covered.
> 
> We need to be able to differentiate code that is stripped out due
> to "version" blocks (which legitimately doesn't count for the
> current build), and code that simply didn't get compiled because
> it WASN'T COVERED.

Good point. But it _is_ valuable to not have unused version blocks not count 
negatively towards code coverage.

- Jonathan M Davis
March 18, 2013
Re: Raising the bar on Phobos unittest coverage
On Monday, March 18, 2013 23:44:15 Martin Nowak wrote:
> On 03/18/2013 09:30 PM, Jonathan M Davis wrote:
> >>> Can someone look into std.file's unittests. They use 60% (32s/50s) of
> >>> 
> >>> > >>the unittest RUNtime on my machine.
> >> > >
> >> > >My first guess would be that you're running a different OS from Walter
> >> > >and
> >> > >that OS-specific code counts as not being run when you run it on a
> >> > >different OS.
> >>
> >>Code that is statically compiled out is not counted as executable code by
> >>the coverage analyzer.
> 
> Sorry for the misunderstanding, but I really meant runtime as in time.
> It's great that we're striving for more unittest coverage but we should
> keep an eye on the time it takes to run them.
> So I ran the executables with /bin/time and std.file accounts for 60% of
> the total runtime (not including compilation). Even though file IO is
> notoriously slow this seems excessive.
> 
> https://github.com/D-Programming-Language/phobos/pull/653

Yeah. The typical approach of testing each function individually is a bit 
expensive with std.file. It really should be refactored so that it does 
everything in only a few unittest blocks rather than a bunch of them so that 
it minimizes how much it repeats I/O operations. I've been meaning to go 
through it and do that but haven't gotten around to it yet. Thanks for 
reminding me though. I'd forgotten about it.

Still, 60% of the total runtime seems awfully high. I don't recall every 
getting the impression that anything like that was going on.

- Jonathan M Davis
March 18, 2013
Re: Raising the bar on Phobos unittest coverage
On Monday, March 18, 2013 23:58:59 Jonas Drewsen wrote:
> On Monday, 18 March 2013 at 22:06:55 UTC, Walter Bright wrote:
> > On 3/18/2013 2:09 PM, Jonas Drewsen wrote:
> >> Please note that std.net.curl needs to have
> >> PHOBOS_TEST_ALLOW_NET env variable
> >> defined or else it will early out on most unittests. I guess
> >> that will increase
> >> the coverage from 2 to something more sane.
> > 
> > Perhaps that should be a -version=TEST_ALLOW_NET setting rather
> > than an environment variable.
> 
> Maybe.. I did it like that so I wouldn't have to recompile
> everytime I needed to change that flag.

The normal thing to do with unit tests when you want to have them enabled on 
part of the time is to use a version block. And you have to recompile whenever 
you make changes anyway, so I wouldn't expect it to be a big deal in general.

- Jonathan M Davis
March 18, 2013
Re: Raising the bar on Phobos unittest coverage
On 3/18/2013 4:08 PM, Jonathan M Davis wrote:
> The normal thing to do with unit tests when you want to have them enabled on
> part of the time is to use a version block. And you have to recompile whenever
> you make changes anyway, so I wouldn't expect it to be a big deal in general.

Also, a unittest executable is built and then run and then deleted. I see no 
point to keeping it around and running it in different ways, hence no real value 
in passing parameters to it via an environment variable.
March 19, 2013
Re: Raising the bar on Phobos unittest coverage
A super critical part of tests that involve networking.. make sure you're 
pointing at resources that you own.  Pointing at even a popular site that 
you're sure can handle the load, is just rude.  The auto-tester currently 
runs the full build/test cycle a little over 1200 times a day these days, 
or roughly once a minute.  Multiply that by however many remote calls it 
makes.

And that's just the tester.. it doesn't include the ad-hoc test runs that 
people do.  Nor does it count whatever auto-testing is done for gdc or 
ldc.

On Mon, 18 Mar 2013, Jonas Drewsen wrote:

> Please note that std.net.curl needs to have PHOBOS_TEST_ALLOW_NET env variable
> defined or else it will early out on most unittests. I guess that will
> increase the coverage from 2 to something more sane.
> 
> -Jonas
> 
> >         $(DMD) -cov=2  -unittest -main -run std\net\curl.d
>
March 19, 2013
Re: Raising the bar on Phobos unittest coverage
On 3/18/2013 5:33 PM, Brad Roberts wrote:
> A super critical part of tests that involve networking.. make sure you're
> pointing at resources that you own.  Pointing at even a popular site that
> you're sure can handle the load, is just rude.  The auto-tester currently
> runs the full build/test cycle a little over 1200 times a day these days,
> or roughly once a minute.  Multiply that by however many remote calls it
> makes.
>
> And that's just the tester.. it doesn't include the ad-hoc test runs that
> people do.  Nor does it count whatever auto-testing is done for gdc or
> ldc.

I agree. We need to comport ourselves as good net citizens.
1 2 3 4
Top | Discussion index | About this forum | D home