On Tue, 2011-01-18 at 09:22 -0500, Kamil Paral wrote:
----- Original Message -----
> This adds the new 'depcheck' test, which checks new updates to ensure
> their dependencies are all met, and sends feedback to Bodhi if so.
Thanks, Will, for your effort on this complicated test!
A few comments:
1. Could you please split depcheck unittest into a separate file?
That would make the whole file much easier to read.
Sure - expect an updated patch set soonish.
2. I see that you're importing _check_already_commented from
bodhi_utils. That method is private. Should we change it to public?
Probably, yes. There's plenty of use cases for it outside of the
bodhi_lib module. But I wanted to keep this patch as self-contained as
possible, which meant avoiding changes outside of tests/depcheck.
PS: bodhi_post_testresult should automatically handle duplicate
comment detection.
Yes, but in order to filter the 'accepted' builds out of the 'pending'
set, we need to check which ones already have 'PASSED' comments in
bodhi. Which means we need to check the comments before the test starts
- this is separate from submitting new results at the end of the test.
3. Could you please print a few status messages inside the test?
For example "Downloading XXX.rpm" messages would be great, so that
we know it's not stuck and what progress is.
Sure, that's no problem.
4. On the other hand, the depcheck script itself is extremely
verbose:
http://kparal.fedorapeople.org/autoqa/depcheck.log
Will the developers be able to identify the source of the dependency
problem? Do we have any hints for them? In this case, the boost package
failed the check.
There's not a clear-cut answer for this question (if there was, we could
automatically fix it!) but looking for the section that says
'PACKAGENAME has depsolving problems' should help:
boost-1.44.0-7.fc14.x86_64 from /boost-1.44.0-7.fc14.x86_64 has
depsolving problems
--> Package: boost-1.44.0-7.fc14.x86_64 (/boost-1.44.0-7.fc14.x86_64)
--> Requires: boost-random = 1.44.0-7.fc14
So where's boost-random? Well, it shows up in the IGNORE line, which
usually means there's already a newer version in the live repos.
Except in this case you've found a bug - boost-random isn't marked as an
upgrade because it's an entirely new package (there's nothing to compare
it to). But that doesn't mean we should drop it from the transaction!
I'll fix this in the updated patch.
5. There was some traceback in my depcheck run (jlaska's patch
applied
and a few printouts added from me), please see log above.
Hrm. Something's not the type I was expecting. I'll check on that.
Further comments below.
> control.autoqa:
> # we want to run this test just for post-koji-build hook
> if hook not in ['post-bodhi-update']:
> execute = False
The comment does not match the code.
Hah oops. Good catch - code is correct, comment is wrong.
> depcheck.py:
> # Get our inputs
> pending = list_pending_updates(kojitag)
> # XXX set testarch to noarch for noarch updates?
> accepted = filter(is_accepted, pending)
This goes through -pending updates, searching for a PASSED comment,
and puts them into ACCEPTED set if they have one. That is I believe
not sufficient, see the comments in here:
http://blogs.fedoraproject.org/wp/wwoods/2011/01/03/depcheck-tags-and-tim...
We have to remember the -pending set in some cachefile, and then
check whether the new -pending set is a superset of the old -pending
set. If it is not, we have to invalidate that set.
I'm not sure about this - how about we revisit this after I fix the more
obvious problems here?
> + # TODO: notify maintainers when an update is rejected - but
ONLY
> once
This seems a little complicated for me. Why don't we simply post test
result to every update in the pending set? Our bodhi_post_testresult()
method automatically cares about comment duplication.
That works for for bodhi comments, but if we still want to notify
maintainers directly - like by email - we'll need to handle that some
other way.
If you need to convert NVR to Bodhi update object (or its title),
this is the approach:
>>> from fedora.client import BodhiClient
>>> bodhi = BodhiClient()
>>> update = bodhi.query(package='sunbird-1.0-0.32.b3pre.fc14')
>>> update['updates'][0]['title']
'sunbird-1.0-0.32.b3pre.fc14,thunderbird-3.1.6-2.fc14'
(let's make a method for it in bodhi_utils).
Good call - do you want to do that, or should I include a second patch
that does this?
-w