-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tests: report passed testcases to Fedora Wiki #601
base: main
Are you sure you want to change the base?
Conversation
083c3f8
to
30ec03a
Compare
30ec03a
to
22beae8
Compare
22beae8
to
54ae12b
Compare
The section is needed when reporting the tests results to the Wiki. Wiki still does not contains Web UI specific tests plans in the Matrix. The sections specified in this commit are not yet present either.
6bae4cc
to
e809bdc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few minor notes. Looks great to me overall.
dist="Fedora", | ||
env=testcase["env"], | ||
milestone="Rawhide", | ||
release="42", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please put the TODO
from commit message to the code. It is more visible.
try: | ||
wiki.login() | ||
except mwclient.errors.LoginError: | ||
wiki.login() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we put at least a small sleep before the second attempt?
Also we should log why was the login unsuccessful if possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, not sure how it would help, we are guessing at this point. Anyway, I copied this code from https://pagure.io/fedora-qa/fedora_openqa/blob/main/f/src/fedora_openqa/report.py#_338
tmpl = "already reported result for test %s, env %s! Will not report dupe." | ||
logger.info(tmpl, dupe.testcase, dupe.env) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that tmpl
variable is necessary, just write the message on two lines or create a new line before.
Just a suggestion, I don't insist on fixing this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it would be long line
logger.debug("full ResTuple: %s", dupe) | ||
|
||
for insuff in insuffs: | ||
tmpl = "insufficient data for test %s, env %s! Will not report." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
Only when testing against composes. This was inspired by openQA Wiki report script [1]. TODO: Remove 'Fedora 42' hardcoded values. [1] https://pagure.io/fedora-qa/fedora_openqa/blob/main/f/src/fedora_openqa/report.py
e809bdc
to
939dd71
Compare
No description provided.