On Tue, 2010-11-09 at 07:44 -0500, Kamil Paral wrote: > ----- "James Laska" <jlaska(a)redhat.com&gt; wrote: > > > > Aaah, I understand the difference and why I was confused by the > > result. > > Thanks for explaining. > > What do you think about this one? Definitely helps me understand the output better. SOme comments below. > =============================================== > >From f9bcda33b036e7df25a50d2b1a4a72c6ef1a0619 Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Kamil=20P=C3=A1ral?= <kparal(a)redhat.com&gt; > Date: Tue, 9 Nov 2010 13:39:41 +0100 > Subject: [PATCH] upgradepath: improve error message clarity > > Use human sentence in the error message instead of a symbolic notation > --- > tests/upgradepath/upgradepath.py | 18 ++++++++++++++---- > 1 files changed, 14 insertions(+), 4 deletions(-) > > diff --git a/tests/upgradepath/upgradepath.py b/tests/upgradepath/upgradepath.py > index edbe4ea..d42e39d 100755 > --- a/tests/upgradepath/upgradepath.py > +++ b/tests/upgradepath/upgradepath.py > @@ -50,9 +50,19 @@ class upgradepath(AutoQATest): > self.outputs = [] > self.highlights = [] > > - def compare(self, matching_build, tags, op, opstr): > + def compare(self, matching_build, tags, op): > koji = autoqa.koji_utils.SimpleKojiClientSession() > result = 'PASSED' > + > + # convert 'op' to human language for error message clarity > + opstr = {operator.eq: 'equal to', > + operator.ge: 'greater or equal than', Maybe rephrase the last part as "... equal to' > + operator.gt: 'greater than', > + operator.le: 'lesser or equal than', Maybe rephrase as "less than or equal to' > + operator.lt: 'lesser than', Same, maybe rephrase the last part as "... equal to'

> + operator.ne: 'not equal to'} > + assert op in opstr, 'Used unsupported operator: %s' % op > + Heh, that's funny. I was looking to see if the operator module itself had some text for each of those functions, so I like your opstr dictionary above.

> for tag in tags: > latest_build = koji.latest_by_tag(tag, matching_build['name']) > if latest_build is None: > @@ -80,7 +90,7 @@ class upgradepath(AutoQATest): > print msg > self.outputs.append(msg) > if not ok: > - msg = '\tFailed condition: Requested package %s Latest package' % opstr > + msg = '\tError: Requested package must be %s the latest package' % opstr[op] Future-proofing ... are there any operators we aren't adding to the opstr dictionary that might result in a traceback at some future date? Should we convert the access to opstr.get(op, "UNKNOWN")?

Or should we stuff an assert in there somewhere (assert optsr.has_key(op))?

> print msg > self.outputs.append(msg) > result = 'FAILED' > @@ -138,9 +148,9 @@ class upgradepath(AutoQATest): > self.highlights.append(msg) > > # compare with lower or equal tags, so version has to be greater or equal > - result = self.compare(matching_build, low_eq_tags, operator.ge, '>=') > + result = self.compare(matching_build, low_eq_tags, operator.ge) > # compare with higher tags, so version has to be lower or equal > - result2 = self.compare(matching_build, hi_tags, operator.le, '<=') > + result2 = self.compare(matching_build, hi_tags, operator.le) > > # compute pkg result > if self.result_order.index(result2) > self.result_order.index(result): > -- > 1.7.3.2 > > ==================================================== > > # autoqa post-bodhi-update --title xxx --target-tag dist-f13-updates GMT-4.5.5-2.fc13 -t upgradepath --local > 13:37:27 INFO | ======================================== > 13:37:27 INFO | GMT-4.5.5-2.fc13 into dist-f13-updates > 13:37:27 INFO | ======================================== > 13:37:29 INFO | [ OK ] dist-f10 > 13:37:29 INFO | Latest package: GMT-4.3.1-2.fc10 > 13:37:29 INFO | [ OK ] dist-f10-updates > 13:37:29 INFO | Latest package: GMT-4.4.0-1.fc10 > 13:37:30 INFO | [ OK ] dist-f11 > 13:37:30 INFO | Latest package: GMT-4.4.0-2.fc11 > 13:37:30 INFO | [ OK ] dist-f11-updates > 13:37:30 INFO | Latest package: GMT-4.5.0-4.fc11 > 13:37:31 INFO | [ OK ] dist-f12 > 13:37:31 INFO | Latest package: GMT-4.5.1-1.fc12 > 13:37:32 INFO | [ OK ] dist-f12-updates > 13:37:32 INFO | Latest package: GMT-4.5.3-3.fc12 > 13:37:32 INFO | [ OK ] dist-f13 > 13:37:32 INFO | Latest package: GMT-4.5.2-1.fc13 > 13:37:33 INFO | [ OK ] dist-f13-updates > 13:37:33 INFO | Latest package: GMT-4.5.3-3.fc13 > 13:37:34 INFO | [FAIL] dist-f14 > 13:37:34 INFO | Latest package: GMT-4.5.3-3.fc14 > 13:37:34 INFO | Error: Requested package must be lesser or equal than the latest package > 13:37:37 INFO | [ OK ] dist-f14-updates > 13:37:37 INFO | No such package: GMT > 13:37:37 INFO | [ OK ] dist-f15 > 13:37:37 INFO | Latest package: GMT-4.5.5-2.fc15 > 13:37:37 INFO | RESULT: FAILED > 13:37:37 INFO | SUMMARY: 1 FAILED > 13:37:37 INFO | **************************************** Looks good to me ... definitely addresses my confusion.