PR#56: Don't resultsdb report EXTRA jobs, add NOREPORT - fedora-qa/fedora_openqa

fedora-qa / fedora_openqa

#56 Don't resultsdb report EXTRA jobs, add NOREPORT

Merged 7 years ago by adamwill. Opened 7 years ago by adamwill.

noreport into master

Don't resultsdb report EXTRA jobs, add NOREPORT

Adam Williamson • 7 years ago

5a7b996

fedora_openqa/report.py

file modified

+13 -4

		`@@ -207,11 +207,12 @@`
		`# it's wikitcms' job to take a compose ID and figure out`
		`# what the validation event for it is.`
		`composeid = job['settings']['BUILD']`
		`- if composeid.endswith('EXTRA'):`
		`+ if composeid.endswith('EXTRA') or composeid.endswith('NOREPORT'):`
		`# this is a 'dirty' test run with extra parameters`
		`- # (usually a test with an updates.img), we never want`
		`- # to report results for these`
		`- logger.debug("Job was run with extra params! Will not report")`
		`+ # (usually a test with an updates.img) or some kind`
		`+ # of throwaway run, we never want to report results`
		`+ # for these`
		`+ logger.debug("Job %d is a NOREPORT job or was run with extra params! Will not report", job['id'])`
		`continue`
		`# find the TESTSUITES entry for the job and parse it to`
		`# get a list of passed test case names (TESTCASES keys)`
		`@@ -353,6 +354,14 @@`
		`logger.warning("cannot report job %d because it is missing build/distri/version", job['id'])`
		`continue`

		`+ if build.endswith('EXTRA') or build.endswith('NOREPORT'):`
		`+ # this is a 'dirty' test run with extra parameters`
		`+ # (usually a test with an updates.img) or some kind`
		`+ # of throwaway run, we never want to report results`
		`+ # for these`
		`+ logger.debug("Job %d is a NOREPORT job or was run with extra params! Will not report", job['id'])`
		`+ continue`
		`+`
		`# sanitize the test name`
		`tc_name = tcname_safeify.sub("_", job['test']).lower()`

tests/test_report.py

file modified

+21 -1

		`@@ -286,6 +286,15 @@`
		`ret = fosreport.get_passed_testcases([jobdict01])`
		`assert len(ret) == 0`

		`+ def test_noreport(self, jobdict01):`
		`+ """Check get_passed_testcases returns nothing when build ends in`
		`+ NOREPORT (intended for use by admins when manually triggering some`
		`+ kind of 'throwaway' test run).`
		`+ """`
		`+ jobdict01['settings']['BUILD'] = 'Fedora-Rawhide-20170207.n.0-NOREPORT'`
		`+ ret = fosreport.get_passed_testcases([jobdict01])`
		`+ assert len(ret) == 0`
		`+`

		`@mock.patch('fedora_openqa.report.get_passed_testcases', return_value=['atest'], autospec=True)`
		`@pytest.mark.usefixtures("oqaclientmock")`
		`@@ -450,7 +459,7 @@`

		`def test_skips(self, fakeres, oqaclientmock):`
		`"""Check resultsdb_report skips reporting for cloned,`
		`- cancelled, obsolete and incomplete jobs.`
		`+ cancelled, obsolete, incomplete, EXTRA and NOREPORT jobs.`
		`"""`
		`jobdict = oqaclientmock[2]`
		`with mock.patch.dict(jobdict, {'clone_id': 15}):`
		`@@ -476,6 +485,17 @@`
		`assert fakeres.call_count == 0`
		`jobdict['settings'][required] = backup`

		`+ fakeres.reset_mock()`
		`+ jobdict['settings']['BUILD'] = 'Fedora-Rawhide-20170207.n.0-EXTRA'`
		`+ fosreport.resultsdb_report(jobs=[1])`
		`+ assert fakeres.call_count == 0`
		`+`
		`+ fakeres.reset_mock()`
		`+ jobdict['settings']['BUILD'] = 'Fedora-Rawhide-20170207.n.0-NOREPORT'`
		`+ fosreport.resultsdb_report(jobs=[1])`
		`+ assert fakeres.call_count == 0`
		`+`
		`+`
		`def test_note(self, fakeres, oqaclientmock):`
		`"""Check resultsdb_report adds a note for failed modules."""`
		`jobdict = oqaclientmock[2]`

adamwill commented 7 years ago

This commit combines a couple of changes related to sometimes
not reporting results.

We've long skipped wiki reporting of results when the job's
'build' value ends in 'EXTRA'; this string is added to the
'build' value when an admin uses the CLI to trigger a set of
tests using an updates image. However, when implementing rdb
reporting, we forgot to do the same there; so this commit adds
that.

The commit also adds another magic string that can be put in
the 'build' value to prevent wiki and resultsdb reporting from
happening: 'NOREPORT'. This is so we can manually trigger
throwaway runs and avoid having the results reported - e.g. I
just ran some tests on an F26 release ISO just to check
something; I didn't want those results sent to rdb or the wiki.
Of course, we could just use a 'build' value ending in EXTRA
in this case too, but I thought having a separate magic string
to use would be a bit better; it avoids any possibility of
confusion between 'completely manually triggered job where we
decided we don't want to report the results' and 'job triggered
via the CLI which used the extraparams mechanism'.

adamwill commented 7 years ago

@jsedlak @pschindl I was just gonna commit this, but then I thought it might be a good one for learning purposes. What actually tends to happen at present is I use some kind of made-up 'build' value for tests like this, and then we get a flood of traceback mails because the fedmsg consumer tries to report the result to resultsdb and blows up along the way, ultimately because resultsdb_conventions relies on the 'build' value being usable as a compose ID.

I don't want to just make the case where fedfind can't handle the 'build' value fail less noisily (by having resultsdb_conventions or fedora_openqa catch the relevant exception and fail gracefully) because in general we do expect all 'normal' openQA results to be reported to resultsdb successfully, and I'd rather we get exception notification emails if we ever get a failure like this when we're not expecting it, rather than just having the failure 'gracefully' handled (and us not notice that results aren't going to resultsdb).

The wiki case is a bit different because we don't expect that all results can be successfully forwarded to the wiki (because of course there isn't always a validation event to report against), so for the wiki case we do just gracefully handle the case where we can't find an event from the 'build' value.