Libguestfs - Aug 2017 - Re: [PATCH 0/2] Add lightweight bindings for PCRE.

On Thu, Aug 03, 2017 at 06:08:24PM +0200, Pino Toscano
wrote:
> But I guess I have no chance to change the current patch (even adding
> a test using asserts, instead of oUnit, for which I still did not get
> a reason why a new test using it would be unacceptable), so ...
Fewer dependencies make the code easier for others to consume, and
that's especially true when (like oUnit) those dependencies add little
value.  (That's different from dependencies like, say, Augeas which
bring huge value.)

So by adding a test with asserts I'm gradually working towards
reducing -- or at least not increasing -- the amount of oUnit being
used.

Practically, someone who compiles libguestfs and runs the tests but
doesn't have oUnit installed will be able to run this test, whereas
the oUnit-based tests would be skipped.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

You asked me to reply to some other points (copied from
https://www.redhat.com/archives/libguestfs/2017-July/msg00189.html):
> 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.
I don't think that every past decision should be unchangable.
> 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.
I think we should remove oUnit tests and replace them with asserts.
But it's not of such critical importance that we need to do that right
now, or even soon.  However adding more oUnit tests means adding more
tests which can't be run unless you have oUnit installed.
> > 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.
[Although it's incidental to this, I should say that in Fedora the
full test suite is enabled, it's just run in two different places (not
Koji because of the complexity and time involved running it in Koji).
‘make check-release’ is run when the tarball is built, and ‘make
installcheck’ is run manually by me after the package has been built.]
> 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.
If you compare an oUnit test:

 
https://github.com/libguestfs/libguestfs/blob/master/common/mlutils/c_utils_unit_tests.ml

to an assert test:

 
https://github.com/libguestfs/libguestfs/blob/master/daemon/daemon_utils_tests.ml

There is barely any difference to me.  This is my point really: oUnit
doesn't bring any particular benefit.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

On Friday, 4 August 2017 10:01:28 CEST Richard W.M. Jones
wrote:
> You asked me to reply to some other points (copied from
> https://www.redhat.com/archives/libguestfs/2017-July/msg00189.html):
> 
> > 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.
> 
> I don't think that every past decision should be unchangable.
I agree, but there should be reasons to switch back, since we already
invested into this.
> > 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.
> 
> I think we should remove oUnit tests and replace them with asserts.
And I strongly beg to disagree here.

Also, I don't understand why this idea is brought to the table at this
point, after I reply (to the limit of annoyingly poking) to get more
information about it.  When I proposed using oUnit I asked about it,
it was deemed good, and then implemented.  I don't see why reversing it
should be done in a subtle way like this.
> But it's not of such critical importance that we need to do that right
> now, or even soon.  However adding more oUnit tests means adding more
> tests which can't be run unless you have oUnit installed.
As I already wrote, the availability of oUnit is not a problem at all,
and neither is its weight.
> > > 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.
> 
> [Although it's incidental to this, I should say that in Fedora the
> full test suite is enabled, it's just run in two different places (not
> Koji because of the complexity and time involved running it in Koji).
OK, but still Fedora falls into the "has oUnit" category, and it's
even
maintained by you.
> ‘make check-release’ is run when the tarball is built, and ‘make
> installcheck’ is run manually by me after the package has been built.]
Both operations are run by whoever does the release, which is you
(using Fedora, and thus with oUnit available).
> > 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.
> 
> If you compare an oUnit test:
> 
>  
https://github.com/libguestfs/libguestfs/blob/master/common/mlutils/c_utils_unit_tests.ml
> 
> to an assert test:
> 
>  
https://github.com/libguestfs/libguestfs/blob/master/daemon/daemon_utils_tests.ml
> 
> There is barely any difference to me.  This is my point really: oUnit
> doesn't bring any particular benefit.
Just few points that come into my mind:
- better reporting for expected/got values
- better interaction for exceptions (e.g. expect/don't expect one)
- logging
- possibility to run stuff in subprocesses (which we don't use,
  although I used it on something private)
- possibility to run only some of the tests, which greatly helps
  debugging the failure of one

All those thigs at least help _me_ when debugging tests, and as such
they provide value to me.

Sure, all the above things can be done manually; what would be the
gain though, other basically rewriting oUnit?

-- 
Pino Toscano