Libguestfs - Jul 2017 - Re: [PATCH 06/27] daemon: Add unit tests of the ‘Utils’ module.

On Wednesday, 19 July 2017 16:33:39 CEST Richard W.M. Jones
wrote:
> On Wed, Jul 19, 2017 at 02:29:33PM +0200, Pino Toscano wrote:
> > On Wednesday, 19 July 2017 14:21:51 CEST Richard W.M. Jones wrote:
> > > On Wed, Jul 19, 2017 at 12:57:08PM +0200, Pino Toscano wrote:
> > > > Would it be possible to use oUnit too?
> > > 
> > > I'm not clear on what benefit oUnit gives us which is worth
the extra
> > > dependency it pulls in.
> > 
> > I was referring to OUnit2, which we already have an optional
dependency
> > for almost all the OCaml tests.
> 
> Sure, understood.  But what would we lose by converting those to
> simple programs that just ran a series of "assert"s?  If a single
> test fails we have to fix it anyway.
The assert means that you will just get the failure, and you will need
to edit the test just to see what was the actual failure.  Sure, it is
doable, but it adds extra steps to debugging.
> If the lack of oUnit2 means that other users are skipping
> those tests, oUnit2 may be a negative.
I'm not sure I follow this.  We already implemented in the past tests
using OUnit (then converted to OUnit2), which means we sort of agreed
to use a (relatively simple) unit test framework.  Most of the OCaml
tests use it already, so the logic that follows here would be to keep
using it.

If the problem is "this test is already sent for review as series of
assert", then I can volunteer to rewrite it to OUnit2.  Otherwise,
I'd like to know what are the reasons against using something that
already discussed in the past, approved, and established.

-- 
Pino Toscano

On Thu, Jul 20, 2017 at 09:48:52AM +0200, Pino Toscano
wrote:
> On Wednesday, 19 July 2017 16:33:39 CEST Richard W.M. Jones wrote:
> > On Wed, Jul 19, 2017 at 02:29:33PM +0200, Pino Toscano wrote:
> > > On Wednesday, 19 July 2017 14:21:51 CEST Richard W.M. Jones
wrote:
> > > > On Wed, Jul 19, 2017 at 12:57:08PM +0200, Pino Toscano
wrote:
> > > > > Would it be possible to use oUnit too?
> > > > 
> > > > I'm not clear on what benefit oUnit gives us which is
worth the extra
> > > > dependency it pulls in.
> > > 
> > > I was referring to OUnit2, which we already have an optional
dependency
> > > for almost all the OCaml tests.
> > 
> > Sure, understood.  But what would we lose by converting those to
> > simple programs that just ran a series of "assert"s?  If a
single
> > test fails we have to fix it anyway.
> 
> The assert means that you will just get the failure, and you will need
> to edit the test just to see what was the actual failure.  Sure, it is
> doable, but it adds extra steps to debugging.
> 
> > If the lack of oUnit2 means that other users are skipping
> > those tests, oUnit2 may be a negative.
> 
> I'm not sure I follow this.  We already implemented in the past tests
> using OUnit (then converted to OUnit2), which means we sort of agreed
> to use a (relatively simple) unit test framework.  Most of the OCaml
> tests use it already, so the logic that follows here would be to keep
> using it.
> 
> If the problem is "this test is already sent for review as series of
> assert", then I can volunteer to rewrite it to OUnit2.  Otherwise,
> I'd like to know what are the reasons against using something that
> already discussed in the past, approved, and established.
My reasoning is that oUnit2 is optional.  If packagers don't have
it and they run the tests then the oUnit2 tests are skipped
(see ‘if HAVE_OCAML_PKG_OUNIT’ in the makefiles).

A simpler test framework which didn't use an external dependency
wouldn't be skipped.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

On Thursday, 20 July 2017 09:59:19 CEST Richard W.M. Jones
wrote:
> On Thu, Jul 20, 2017 at 09:48:52AM +0200, Pino Toscano wrote:
> > On Wednesday, 19 July 2017 16:33:39 CEST Richard W.M. Jones wrote:
> > > On Wed, Jul 19, 2017 at 02:29:33PM +0200, Pino Toscano wrote:
> > > > On Wednesday, 19 July 2017 14:21:51 CEST Richard W.M. Jones
wrote:
> > > > > On Wed, Jul 19, 2017 at 12:57:08PM +0200, Pino Toscano
wrote:
> > > > > > Would it be possible to use oUnit too?
> > > > > 
> > > > > I'm not clear on what benefit oUnit gives us which
is worth the extra
> > > > > dependency it pulls in.
> > > > 
> > > > I was referring to OUnit2, which we already have an optional
dependency
> > > > for almost all the OCaml tests.
> > > 
> > > Sure, understood.  But what would we lose by converting those to
> > > simple programs that just ran a series of "assert"s? 
If a single
> > > test fails we have to fix it anyway.
> > 
> > The assert means that you will just get the failure, and you will need
> > to edit the test just to see what was the actual failure.  Sure, it is
> > doable, but it adds extra steps to debugging.
> > 
> > > If the lack of oUnit2 means that other users are skipping
> > > those tests, oUnit2 may be a negative.
> > 
> > I'm not sure I follow this.  We already implemented in the past
tests
> > using OUnit (then converted to OUnit2), which means we sort of agreed
> > to use a (relatively simple) unit test framework.  Most of the OCaml
> > tests use it already, so the logic that follows here would be to keep
> > using it.
> > 
> > If the problem is "this test is already sent for review as series
of
> > assert", then I can volunteer to rewrite it to OUnit2. 
Otherwise,
> > I'd like to know what are the reasons against using something that
> > already discussed in the past, approved, and established.
> 
> My reasoning is that oUnit2 is optional.
Sure.  OTOH, we already do use oUnit2, so these reasonings were already
discussed in the past, and (considered we have tests based on oUnit2)
deemed not a problem.

What I still don't understand is why adding a new test based on oUnit2
would be a problem, while the rest of the tests are fine as they are.
> If packagers don't have it and they run the tests then the oUnit2
> tests are skipped (see ‘if HAVE_OCAML_PKG_OUNIT’ in the makefiles).
Packagers rarely run our unit tests, since they make the build times
much longer, even on fast builders.  (Not to mention that sometimes
they use VMs, and thus this adds more issues to that.)  Even in Fedora
the general test suite is disabled, only `make quickcheck` is run.
This means it is mostly us developers (or CI) building and testing the
full test suite.

Regarding the availability: oUnit2 is available in the latest stable
versions of all the major distros, even in older versions.
> A simpler test framework which didn't use an external dependency
> wouldn't be skipped.
Surely I'm not an expert OCaml hacker, but at least to me oUnit2 does
not seem that complex, and a manually written framework would end up
reimplementing most of it, in the end.

-- 
Pino Toscano