----- "James Laska" jlaska@redhat.com wrote:
Basically, I think the upgradepath test below is correctly highlighting a failure, but the text associated with the error is wrong. The test is triggered because of a bodhi request for GMT-4.5.5-2.fc13 into dist-f13-updates-testing. The test fails giving the following reason ...
[FAIL] dist-f14 Latest package: GMT-4.5.3-3.fc14 Failed condition: Requested package <= Latest package
Well... not really :)
There are two ways how to read that error message. The first one (mine, in the time of writing) reads:
"Failed condition: *expected state*"
The second one (yours) reads:
"Failed condition: *current state*"
The logic is reversed, I am trying to state which condition is broken (and state it how it should look when it is correct), you are trying to read out what's currently wrong with it.
I knew this could cause some confusion, but I didn't want to spend more time rewriting application logic. But now I'm convinced we should clarify this more. I'll post a patch where I will change the symbolic equation ('>') to a sentence ('foo must be greater or equal then bar'). That should do it.
On Tue, 2010-11-09 at 04:32 -0500, Kamil Paral wrote:
----- "James Laska" jlaska@redhat.com wrote:
Basically, I think the upgradepath test below is correctly highlighting a failure, but the text associated with the error is wrong. The test is triggered because of a bodhi request for GMT-4.5.5-2.fc13 into dist-f13-updates-testing. The test fails giving the following reason ...
[FAIL] dist-f14 Latest package: GMT-4.5.3-3.fc14 Failed condition: Requested package <= Latest package
Well... not really :)
There are two ways how to read that error message. The first one (mine, in the time of writing) reads:
"Failed condition: *expected state*"
The second one (yours) reads:
"Failed condition: *current state*"
The logic is reversed, I am trying to state which condition is broken (and state it how it should look when it is correct), you are trying to read out what's currently wrong with it.
I knew this could cause some confusion, but I didn't want to spend more time rewriting application logic. But now I'm convinced we should clarify this more. I'll post a patch where I will change the symbolic equation ('>') to a sentence ('foo must be greater or equal then bar'). That should do it.
Aaah, I understand the difference and why I was confused by the result. Thanks for explaining.
Thanks, James
----- "James Laska" jlaska@redhat.com wrote:
Aaah, I understand the difference and why I was confused by the result. Thanks for explaining.
What do you think about this one?
===============================================
From f9bcda33b036e7df25a50d2b1a4a72c6ef1a0619 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kamil=20P=C3=A1ral?= kparal@redhat.com 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', + operator.gt: 'greater than', + operator.le: 'lesser or equal than', + operator.lt: 'lesser than', + operator.ne: 'not equal to'} + assert op in opstr, 'Used unsupported operator: %s' % op + 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] 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):
On Tue, 2010-11-09 at 07:44 -0500, Kamil Paral wrote:
----- "James Laska" jlaska@redhat.com 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@redhat.com 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 clarityopstr = {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.
Thanks, James
autoqa-devel@lists.fedorahosted.org
-
James Laska -
Kamil Paral