[Libguestfs] [PATCH 0/2] Add lightweight bindings for PCRE.

[Pino Toscano]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20170804/408e987c/attachment.sig>