[pacman-dev] [PATCH 3/3] Added tests for -Q --check (both fast(files) and full(mtree)).

[Jeremy Heiner]

On Fri, Oct 4, 2013 at 11:22 AM, Andrew Gregory
<andrew.gregory.8 at gmail.com> wrote:
> There is A LOT going on in this patch.  Please try to break your
> patches up into smaller changes.  At least mtree and hook support
> definitely should have been individual patches.

Hi, Andrew,
Thanks for the code review!
Sorry about the size of this patch. It's really due to the fact that
these things grew organically. If I had been known before I started
exactly what I needed from the mtree and hook support in order to meet
my goals, well, then I would have won several lotteries by now. I'd be
happy to reverse-engineer some additional commits to make it look like
I was more prescient :).

> Before I get into specifics, pacman already has "hooks" of a sort:
> install scripts.  I think I would prefer to see support added for
> install scripts in local db packages than the ability to run arbitrary
> python code.  You could then modify files as needed from
> post_install().  This would match our existing use of install scripts
> as hooks and could be run within fakechroot.  Anybody else have any
> thoughts on this?

I really like that idea. The reason I didn't go down that path was
that I saw what pmpkg.install_package contained, or rather did not
contain, and was scared off.

> The only reason to refactor this code into functions seems to be so
> that you can run pacman from genhook, but you only do that to install
> packages, which should actually be done by adding them to the local
> db, so I'm not really sure what the point of this is.  Furthermore,
> I'm not sure how wise it is to use pacman to set up its own test
> environment.

Testing -Qk(k) requires /var/lib/pacman/local/* to be populated, which
is not something pmpkg does. So how much more does pmpkg need to do?
The more it duplicates what pacman does, the higher the ongoing code
maintenance cost. So where's the sweet spot?

I would argue that pmpkg should do the minimum (i.e. only what it
currently does). And that there is no danger in using pacman to set up
its own test environment because there are extensive unit tests for
the installation features. And if those pass, then where is the
danger? It might be nice to add "this test should fail without even
running unless that test has succeeded" to the specifications, but
that seems a bit overkill.

One more subtle thing to keep in mind is the timing of the snapshot.
Now I didn't really dig deeply into how all the tests make use of the
snapshot, but for some tests you want it taken between the install and
the mangling. There is no way to achieve this effect using local
pmpkgs.

But I want to say again that I really like the idea. There's no
question it is superior to my hooks mechanism in the complexity of the
testing framework and even the generality. I like it so much that I'm
going to delay responding to the other points from your code review to
provide an opportunity for feedback. I think it should be possible to
find workarounds for the issues I mentioned above, and if that's the
case then I would happily abandon my hooks idea in favor of install
scripts.
Thanks again,
Jeremy