Re: Observing low test-suite coverage

[Илья Шипицин]

k, can we review this sometimes ? ))

I'd like to set automatic coverage after that.

ср, 23 февр. 2022 г. в 15:44, Tim Düsterhus :

> Willy,
>
> On 2/23/22 11:43, Илья Шипицин wrote:
> > Willy, can you please apply patch from Tim  (below) ?
>
> No, please don't. This patch is hugely unsafe. Someone will need to
> create a proper patch that wasn't hacked together in 2 minutes.
>
> Best regards
> Tim Düsterhus
>

Re: Observing low test-suite coverage

[Tim Düsterhus]

Willy,

On 2/23/22 11:43, Илья Шипицин wrote:

Willy, can you please apply patch from Tim  (below) ?


No, please don't. This patch is hugely unsafe. Someone will need to 
create a proper patch that wasn't hacked together in 2 minutes.


Best regards
Tim Düsterhus

Re: Observing low test-suite coverage

[Илья Шипицин]

Willy, can you please apply patch from Tim  (below) ?

пт, 4 февр. 2022 г. в 03:06, Tim Düsterhus :

> Hugo,
>
> On 1/25/22 13:13, Hugo Lefeuvre wrote:
> > We are wondering if this is caused by our measurement approach (gcov,
> > passing -fprofile-arcs -ftest-coverage in the CFLAGS and -lgcov to
> > LDFLAGS), or if this is known to the HAProxy community. We reproduced
> these
> > measurements across several recent versions of HAProxy, dev, 2.5, and
> 2.4.
>
> Yes, I believe your measurement is flawed. I tried to reproduce the
> results myself and was confused by seeing uri_normalizer.c not being
> covered when it should be completely covered, except for error paths by
> this test:
>
>
> https://github.com/haproxy/haproxy/blob/master/reg-tests/http-rules/normalize_uri.vtc
>
> Cleaning out all the coverage files and just running that specific test
> using:
>
> vtest -k -t 1 reg-tests/http-rules/normalize_uri.vtc
>
> did not generate any new coverage files!
>
> Digging into it, this appears to be caused by VTest terminating HAProxy
> with a SIGINT, but HAProxy not binding an explicit SIGINT handler. In
> this case it appears that no coverage files are generated.
>
> To test this further I applied the following patch to exit as cleanly as
> possible on SIGINT:
>
> > diff --git i/src/haproxy.c w/src/haproxy.c
> > index f10af5eae..38614a0cb 100644
> > --- i/src/haproxy.c
> > +++ w/src/haproxy.c
> > @@ -790,6 +790,13 @@ void mworker_reload()
> > mworker_reexec();
> >  }
> >
> > +void die_on_sigint(struct sig_handler *sh)
> > +{
> > +   int sig = sh->arg;
> > +
> > +   deinit_and_exit(0);
> > +}
> > +
> >  static void mworker_loop()
> >  {
> >
> > @@ -3424,6 +3431,10 @@ int main(int argc, char **argv)
> > /* when multithreading we need to let only the thread 0 handle
> the signals */
> > haproxy_unblock_signals();
> >
> > +
> > +   signal_register_fct(SIGINT, die_on_sigint, SIGINT);
> > +
> > +
> > /* Finally, start the poll loop for the first thread */
> > run_thread_poll_loop(&ha_thread_info[0]);
>
> and then reran the test suite. A few tests failed (this might be because
> my patch inserted the deinit in the wrong location), but the coverage
> looks much better:
>
> I'm getting ~44% line and ~55% function coverage when compiling with
>
> $ make -j4 all TARGET=linux-glibc V=1
>
> uri_normalizer.c gets to 100% function and 85.4% line coverage. The
> non-covered lines are the error handling (incidentally the call to
> 'my_unreachable()' also is marked as uncovered - which is expected).
>
> Best regards
> Tim Düsterhus
>
>

Re: Observing low test-suite coverage

[Tim Düsterhus]

Hugo,

On 1/25/22 13:13, Hugo Lefeuvre wrote:

We are wondering if this is caused by our measurement approach (gcov,
passing -fprofile-arcs -ftest-coverage in the CFLAGS and -lgcov to
LDFLAGS), or if this is known to the HAProxy community. We reproduced these
measurements across several recent versions of HAProxy, dev, 2.5, and 2.4.


Yes, I believe your measurement is flawed. I tried to reproduce the 
results myself and was confused by seeing uri_normalizer.c not being 
covered when it should be completely covered, except for error paths by 
this test:


https://github.com/haproxy/haproxy/blob/master/reg-tests/http-rules/normalize_uri.vtc

Cleaning out all the coverage files and just running that specific test 
using:


vtest -k -t 1 reg-tests/http-rules/normalize_uri.vtc

did not generate any new coverage files!

Digging into it, this appears to be caused by VTest terminating HAProxy 
with a SIGINT, but HAProxy not binding an explicit SIGINT handler. In 
this case it appears that no coverage files are generated.


To test this further I applied the following patch to exit as cleanly as 
possible on SIGINT:



diff --git i/src/haproxy.c w/src/haproxy.c
index f10af5eae..38614a0cb 100644
--- i/src/haproxy.c
+++ w/src/haproxy.c
@@ -790,6 +790,13 @@ void mworker_reload()
mworker_reexec();
 }
 
+void die_on_sigint(struct sig_handler *sh)

+{
+   int sig = sh->arg;
+
+   deinit_and_exit(0);
+}
+
 static void mworker_loop()
 {
 
@@ -3424,6 +3431,10 @@ int main(int argc, char **argv)

/* when multithreading we need to let only the thread 0 handle the 
signals */
haproxy_unblock_signals();
 
+

+   signal_register_fct(SIGINT, die_on_sigint, SIGINT);
+
+
/* Finally, start the poll loop for the first thread */
run_thread_poll_loop(&ha_thread_info[0]);


and then reran the test suite. A few tests failed (this might be because 
my patch inserted the deinit in the wrong location), but the coverage 
looks much better:


I'm getting ~44% line and ~55% function coverage when compiling with

$ make -j4 all TARGET=linux-glibc V=1

uri_normalizer.c gets to 100% function and 85.4% line coverage. The 
non-covered lines are the error handling (incidentally the call to 
'my_unreachable()' also is marked as uncovered - which is expected).


Best regards
Tim Düsterhus

Re: Observing low test-suite coverage

[Илья Шипицин]

ср, 2 февр. 2022 г. в 21:54, Hugo Lefeuvre :

> Hi,
>
> On Mon, Jan 31, 2022 at 02:25:39PM +0500, Илья Шипицин wrote:
> > can you share details how did you invoked "gcov" ?
> > I tried to make it work recently chipitsine/haproxy | Coveralls - Test
> > Coverage History & Statistics
> >  , but it needs more
> > attention.
>
> 1. Set -fprofile-arcs -ftest-coverage in the CFLAGS and -lgcov in the
> LDFLAGS
> 2. Build and run the regression test suite:
>make TARGET=linux-glibc USE_OPENSSL=1 USE_PCRE=1 USE_SYSTEMD=1 USE_LUA=1
>make reg-tests
> 3. Generate lcov and gcov data:
>cov --capture --directory . --output-file cov.info
>genhtml cov.info --output-directory out_html --demangle-cpp --legend \
> --title "Haproxy Testsuite"
>
> This should output the coverage in HTML format.
>

thanks, I'll try


>
> Best,
> Hugo
>
> --
> Hugo Lefeuvre (hle)|www.owl.eu.com
> RSA4096_ 360B 03B3 BF27 4F4D 7A3F D5E8 14AA 1EB8 A247 3DFD
> ed25519_ 37B2 6D38 0B25 B8A2 6B9F 3A65 A36F 5357 5F2D DC4C
>

Re: Observing low test-suite coverage

[Hugo Lefeuvre]

Hi,

On Mon, Jan 31, 2022 at 02:25:39PM +0500, Илья Шипицин wrote:
> can you share details how did you invoked "gcov" ?
> I tried to make it work recently chipitsine/haproxy | Coveralls - Test
> Coverage History & Statistics
>  , but it needs more
> attention.

1. Set -fprofile-arcs -ftest-coverage in the CFLAGS and -lgcov in the LDFLAGS
2. Build and run the regression test suite:
   make TARGET=linux-glibc USE_OPENSSL=1 USE_PCRE=1 USE_SYSTEMD=1 USE_LUA=1
   make reg-tests
3. Generate lcov and gcov data:
   cov --capture --directory . --output-file cov.info
   genhtml cov.info --output-directory out_html --demangle-cpp --legend \
--title "Haproxy Testsuite"

This should output the coverage in HTML format.

Best,
Hugo

-- 
Hugo Lefeuvre (hle)|www.owl.eu.com
RSA4096_ 360B 03B3 BF27 4F4D 7A3F D5E8 14AA 1EB8 A247 3DFD
ed25519_ 37B2 6D38 0B25 B8A2 6B9F 3A65 A36F 5357 5F2D DC4C


signature.asc
Description: PGP signature

Re: Observing low test-suite coverage

[Hugo Lefeuvre]

Hi Willy,

On Mon, Jan 31, 2022 at 08:47:58AM +0100, Willy Tarreau wrote:
> Just to be sure, what test suite are you talking about ? I guess you mean
> the regression tests in the "reg-tests" directory, but I'm not sure. If so,
> we're well aware that they are still fairly limited, and were introduced
> relatively recently. They're progressively being completed as new features
> are added, but it's for sure that a number of areas of the code are not yet
> covered by them. However we tried to make sure that most of them depend on
> the most sensitive areas that are easily subject to break (and during
> development, there's no single day a developer doesn't break a few regtests
> locally during testing, which indicates that they're representative enough
> for our purpose for now).

I am indeed talking about the regression tests under "reg-tests".

> Also I don't know how the ratio of lines of code is counted in your case.
> We do have quite a bunch of code that is platform-specific or that depends
> on build options, and in this case I have no idea how that is counted. Also

The ratio only considers code compiled in, so platform code or optional
code that's not enabled in the build configuration will not be considered.

> a significant part of the code is dedicated to error handling and will never
> be triggered by regtests. Events like I/O errors, out-of-memory errors and
> unexpected conditions must never happen, yet collectively they probably
> represent about half of the code. So without more details it's hard to
> have a solid opinion on the subject.

This matches our observations. I'd say that the main causes for low
coverage are (in order of importance) the large number of features, many of
which (HAProxy CLI, Lua Scripting, many protocols, admin/debug features)
have low to nonexistent coverage, followed by error cases, and finally the
different implementations of same building blocks (load balancing algos,
polling backends, etc.).

Thanks for your detailed answer!

Best,
Hugo

-- 
Hugo Lefeuvre (hle)|www.owl.eu.com
RSA4096_ 360B 03B3 BF27 4F4D 7A3F D5E8 14AA 1EB8 A247 3DFD
ed25519_ 37B2 6D38 0B25 B8A2 6B9F 3A65 A36F 5357 5F2D DC4C


signature.asc
Description: PGP signature

Re: Observing low test-suite coverage

[Илья Шипицин]

Hugo,

can you share details how did you invoked "gcov" ?
I tried to make it work recently chipitsine/haproxy | Coveralls - Test
Coverage History & Statistics
 , but it needs more
attention.


вт, 25 янв. 2022 г. в 17:16, Hugo Lefeuvre :

> Hi!
>
> As part of research project, my colleagues and myself are measuring the
> test-suite coverage of HAProxy using gcov. We found the coverage numbers to
> be relatively low compared to other major cloud applications, capping at
> 13-14% line coverage (other projects such as Nginx, Redis, etc. are more in
> the order of 60-90%).
>
> We are wondering if this is caused by our measurement approach (gcov,
> passing -fprofile-arcs -ftest-coverage in the CFLAGS and -lgcov to
> LDFLAGS), or if this is known to the HAProxy community. We reproduced these
> measurements across several recent versions of HAProxy, dev, 2.5, and 2.4.
>
> Thanks for your work!
>
> Best,
> Hugo
>
> --
> Hugo Lefeuvre (hle)|www.owl.eu.com
> RSA4096_ 360B 03B3 BF27 4F4D 7A3F D5E8 14AA 1EB8 A247 3DFD
> ed25519_ 37B2 6D38 0B25 B8A2 6B9F 3A65 A36F 5357 5F2D DC4C
>

Re: Observing low test-suite coverage

[Willy Tarreau]

Hi Hugo,

On Tue, Jan 25, 2022 at 12:13:16PM +, Hugo Lefeuvre wrote:
> Hi!
> 
> As part of research project, my colleagues and myself are measuring the
> test-suite coverage of HAProxy using gcov. We found the coverage numbers to
> be relatively low compared to other major cloud applications, capping at
> 13-14% line coverage (other projects such as Nginx, Redis, etc. are more in
> the order of 60-90%).
> 
> We are wondering if this is caused by our measurement approach (gcov,
> passing -fprofile-arcs -ftest-coverage in the CFLAGS and -lgcov to
> LDFLAGS), or if this is known to the HAProxy community. We reproduced these
> measurements across several recent versions of HAProxy, dev, 2.5, and 2.4.

Just to be sure, what test suite are you talking about ? I guess you mean
the regression tests in the "reg-tests" directory, but I'm not sure. If so,
we're well aware that they are still fairly limited, and were introduced
relatively recently. They're progressively being completed as new features
are added, but it's for sure that a number of areas of the code are not yet
covered by them. However we tried to make sure that most of them depend on
the most sensitive areas that are easily subject to break (and during
development, there's no single day a developer doesn't break a few regtests
locally during testing, which indicates that they're representative enough
for our purpose for now).

Also I don't know how the ratio of lines of code is counted in your case.
We do have quite a bunch of code that is platform-specific or that depends
on build options, and in this case I have no idea how that is counted. Also
a significant part of the code is dedicated to error handling and will never
be triggered by regtests. Events like I/O errors, out-of-memory errors and
unexpected conditions must never happen, yet collectively they probably
represent about half of the code. So without more details it's hard to
have a solid opinion on the subject.

Regards,
Willy