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:
- 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.
- 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.
- 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.
- 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.
- 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-timing...
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