libtaskotron/directives/resultsdb_directive.py
25	Would this be clearer? You must use either ``results`` or ``file`` parameter (exactly one).
77	`on on`
77–85	This seems to be part of the first example (the first two directives), but it's a separate example. Let's put a new paragraph between the first two and the second two directives. Let's say something like "Read ResultsYAML from a file, as saved by a bash check".
84	Typo. But actually, do we need to export the output? I think we used it for Bodhi reporting in the past, now we don't need it at all? Maybe we can strip it from the examples.
262–263	Would it make sense to wrap this as well and raise `TaskotronDirectiveError` for IO errors? Or do we want to raise them directly (maybe document them then)?
268–269	Maybe get rid of the apostrophes in the message, because this is going to be printed even for `file:` now.
testing/test_resultsdb_directive.py
183–184	This makes me very uneasy. Why we need to mock builtin `open`? That's a big freaking hammer. Would it be better to use pytest's `tmpdir`, as we use it in `functest_yumrepoinfo_directive.py`, to create a fake file with predefined contents (therefore ideally put it into `functest_resultsdb_directive`)?

testing/test_resultsdb_directive.py
183–184	Taken from xunit directive, tbh. I don't really see the problem of mocking `open()`, though, feels even cleaner than creating fake files to me.

libtaskotron/directives/resultsdb_directive.py
261–269	IIUIC, this didn't fix the issue unless you also start catching `IOError`.
testing/test_resultsdb_directive.py
183–184	I don't like overriding builtins at all, but I won't fight it at this point. However, according to this and this it might be a better idea to patch `open` just for the current module, and not globally in `__builtin__` space.

libtaskotron/directives/resultsdb_directive.py
258–259	Also, please add this to `raises:` in documentation, thanks.

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	2400	3a38e6d		Jul 21 2016, 2:08 PM	★	★
Diff 2	2401	3a38e6d	- fix docs	Jul 21 2016, 2:14 PM	★	★
Diff 3	2409	3a38e6d	- review	Jul 22 2016, 11:04 AM	★	★
Diff 4	2412	3a38e6d	- make @kparal happy panda	Jul 25 2016, 8:39 AM	★	★
Diff 5	2414	679736d	rLTRNd9b7a64c5b9afa7553743fed5bbb5f809b9d17f0	Jul 25 2016, 9:15 AM	★	★

Commit	Tree	Parents	Author	Summary	Date
f24c0efa3a2f	c42a2ae57ee8	1f551b2c9b15	Josef Skladanka	lint	Jul 25 2016, 8:39 AM
1f551b2c9b15	cb17a1af36ac	6e559323dd78	Josef Skladanka	.	Jul 25 2016, 8:37 AM
6e559323dd78	55d46e5663e6	8d6bc07d3529	Josef Skladanka	typo	Jul 22 2016, 11:04 AM
8d6bc07d3529	83d5073a68e1	bf440a4a4404	Josef Skladanka	review	Jul 22 2016, 11:03 AM
bf440a4a4404	537f0a5b15aa	3a38e6dbd9a4	Josef Skladanka	Allow resultsdb_directive to read from file (Show More…)	Jul 22 2016, 10:22 AM

Diff 2414

libtaskotron/directives/resultsdb_directive.py

1	# -- coding: utf-8 --	1		# -- coding: utf-8 --
2	# Copyright 2009-2014, Red Hat, Inc.	2		# Copyright 2009-2014, Red Hat, Inc.
3	# License: GPL-2.0+ <http://spdx.org/licenses/GPL-2.0+>	3		# License: GPL-2.0+ <http://spdx.org/licenses/GPL-2.0+>
4	# See the LICENSE file for more details on Licensing	4		# See the LICENSE file for more details on Licensing
5		5
6	from __future__ import absolute_import	6		from __future__ import absolute_import
7		7
8	DOCUMENTATION = """	8		DOCUMENTATION = """
9	module: resultsdb_directive	9		module: resultsdb_directive
10	short_description: send task results to ResultsDB or check YAML correctness	10		short_description: send task results to ResultsDB or check YAML correctness
11	description: \|	11		description: \|
12	Send check output to `ResultsDB <https://fedoraproject.org/wiki/ResultsDB>`_.	12		Send check output to `ResultsDB <https://fedoraproject.org/wiki/ResultsDB>`_.
13		13
14	If reporting to ResultsDB is disabled in the config (`report_to_resultsdb`	14		If reporting to ResultsDB is disabled in the config (``report_to_resultsdb``
15	option, disabled by default for the development profile), the directive at	15		option, disabled by default for the development profile), the directive at
16	least checks whether the check output is in valid :ref:`resultyaml-format` and logs the	16		least checks whether the check output is in valid :ref:`resultyaml-format` and logs the
17	details that would have been reported. For this reason, it's useful to use	17		details that would have been reported. For this reason, it's useful to use
18	resultsdb directive always, even when you don't have any ResultsDB server	18		resultsdb directive always, even when you don't have any ResultsDB server
19	configured.	19		configured.
20		20
21	parameters:	21		parameters:
22	results:	22		results:
23	required: true	23		required: true
24	description: The output of the check in the ResultYAML format	24		description: Variable containing the results of the check in the ResultYAML
		25		format. You must use either ``results`` or ``file`` parameter.
			kparalUnsubmitted Not Done Would this be clearer? You must use either ``results`` or ``file`` parameter (exactly one).
		26		type: str
		27		file:
		28		required: true
		29		description: Path to a file containing the results of the check in the
		30		ResultYAML format.
		31		You must use either ``results`` or ``file`` parameter.
25	type: str	32		type: str
26	returns: \|	33		returns: \|
27	YAML formatted output as taken from input, enhanced with result-id's for each	34		YAML formatted output as taken from input, enhanced with result-id's for each
28	successfully stored result. The result-id information then can be used by	35		successfully stored result. The result-id information then can be used by
29	other directives. If reporting to ResultsDB is disabled, the returned YAML	36		other directives. If reporting to ResultsDB is disabled, the returned YAML
30	does not contain this extra field.	37		does not contain this extra field.
31	raises: \|	38		raises: \|
32	* :class:`.TaskotronDirectiveError`: if ``results`` is not in a valid YAML	39		* :class:`.TaskotronDirectiveError`: if ``results`` is not in a valid YAML
33	format, or when ``item`` or ``type`` are missing in the YAML's result data,	40		format, or when ``item`` or ``type`` are missing in the YAML's result data,
34	or when results are not allowed to be posted into a given namespace, or	41		or when results are not allowed to be posted into a given namespace, or
35	the namespace does not exist.	42		the namespace does not exist, or if both ``results`` and ``file`` parameters
		43		are used;
36	* :class:`resultsdb_api.ResultsDBapiException`: for any errors coming from	44		* :class:`resultsdb_api.ResultsDBapiException`: for any errors coming from
37	the ResultsDB server	45		the ResultsDB server
38	version_added: 0.4	46		version_added: 0.4
39	"""	47		"""
40		48
41	EXAMPLES = """	49		EXAMPLES = """
42	These two actions first run ``run_rpmlint.py`` and export its YAML output to	50		These two actions first run ``run_rpmlint.py`` and export its YAML output to
43	``${rpmlint_output}``, and then feed this YAML output to ``resultsdb``	51		``${rpmlint_output}``, and then feed this YAML output to ``resultsdb``
44	directive::	52		directive::
45		53
46	- name: run rpmlint on downloaded rpms	54		- name: run rpmlint on downloaded rpms
47	python:	55		python:
48	file: run_rpmlint.py	56		file: run_rpmlint.py
49	callable: run	57		callable: run
50	workdir: ${workdir}	58		workdir: ${workdir}
51	export: rpmlint_output	59		export: rpmlint_output
52		60
53	- name: report results to resultsdb	61		- name: report results to resultsdb
54	resultsdb:	62		resultsdb:
55	results: ${rpmlint_output}	63		results: ${rpmlint_output}
56	export: resultsdb_output	64
		65		This example shows using the ``file`` parameter. The ``randomtest`` produces
		66		a ``testoutput.yml`` file containing results in the YAML format, that the
		67		``resultsdb`` directive then uses::
		68
		69		- name: run randomtest on downloaded rpms
		70		shell:
		71		- bash ./randomtest.sh --outfile=${workdir}/testoutput.yml
		72
		73		- name report results to resultsdb
		74		resultsdb:
		75		file: ${workdir}/testoutput.yml
57		76
58	If ResultsDB reporting is configured in the config file, it will get saved on	77		If ResultsDB reporting is configured in the config file, it will get saved on
			kparalUnsubmitted Not Done `on on`
59	the ResultsDB server, otherwise only YAML compliance will get checked and some	78		the ResultsDB server, otherwise only YAML compliance will get checked and some
60	summary will be printed out into the log, like this::	79		summary will be printed out into the log, like this::
61		80
62	[libtaskotron:resultsdb_directive.py:143] 2014-06-24 13:55:27 INFO YAML is OK.	81		[libtaskotron:resultsdb_directive.py:143] 2014-06-24 13:55:27 INFO YAML is OK.
63	[libtaskotron:resultsdb_directive.py:144] 2014-06-24 13:55:27 INFO Reporting to ResultsDB is disabled.	82		[libtaskotron:resultsdb_directive.py:144] 2014-06-24 13:55:27 INFO Reporting to ResultsDB is disabled.
64	[libtaskotron:resultsdb_directive.py:145] 2014-06-24 13:55:27 INFO Once enabled, the following would be reported:	83		[libtaskotron:resultsdb_directive.py:145] 2014-06-24 13:55:27 INFO Once enabled, the following would be reported:
65	[libtaskotron:resultsdb_directive.py:146] 2014-06-24 13:55:27 INFO <CheckDetail: {'_outcome': 'PASSED',	84		[libtaskotron:resultsdb_directive.py:146] 2014-06-24 13:55:27 INFO <CheckDetail: {'_outcome': 'PASSED',
			kparalUnsubmitted Not Done Typo. But actually, do we need to export the output? I think we used it for Bodhi reporting in the past, now we don't need it at all? Maybe we can strip it from the examples.
66	'item': 'xchat-2.8.8-21.fc20.x86_64.rpm',	85		'item': 'xchat-2.8.8-21.fc20.x86_64.rpm',
			kparalUnsubmitted Not Done This seems to be part of the first example (the first two directives), but it's a separate example. Let's put a new paragraph between the first two and the second two directives. Let's say something like "Read ResultsYAML from a file, as saved by a bash check".
67	'keyvals': {},	86		'keyvals': {},
68	'output': '<stripped out>',	87		'output': '<stripped out>',
69	'report_type': 'koji_build',	88		'report_type': 'koji_build',
70	'summary': 'RPMLINT PASSED for xchat-2.8.8-21.fc20.x86_64.rpm'}>	89		'summary': 'RPMLINT PASSED for xchat-2.8.8-21.fc20.x86_64.rpm'}>
71	<CheckDetail: {'_outcome': 'PASSED',	90		<CheckDetail: {'_outcome': 'PASSED',
72	'item': 'xchat-tcl-2.8.8-21.fc20.x86_64.rpm',	91		'item': 'xchat-tcl-2.8.8-21.fc20.x86_64.rpm',
73	'keyvals': {},	92		'keyvals': {},
74	'output': '<stripped out>',	93		'output': '<stripped out>',
▲ Show 20 Lines • Show All 155 Lines • ▼ Show 20 Line(s)		206	def check_namespace(self, checkname, arg_data):
230	except TaskotronDirectiveError:	249		except TaskotronDirectiveError:
231	raise TaskotronDirectiveError("%s is not allowed to post results into %s "	250		raise TaskotronDirectiveError("%s is not allowed to post results into %s "
232	"namespace. Not posting results." %	251		"namespace. Not posting results." %
233	(arg_data['checkname'], arg_data['namespace']))	252		(arg_data['checkname'], arg_data['namespace']))
234		253
235	def process(self, params, arg_data):	254		def process(self, params, arg_data):
236	# checking if reporting is enabled is done after importing yaml which	255		# checking if reporting is enabled is done after importing yaml which
237	# serves as validation of input results	256		# serves as validation of input results
		257
		258		if 'file' in params and 'results' in params:
		259		raise TaskotronDirectiveError("Either `file` or `results` can be used, not both.")
			kparalUnsubmitted Not Done Also, please add this to `raises:` in documentation, thanks.
		260
238	try:	261		try:
		262		if params.get('file', None):
		263		with open(params['file']) as resultfile:
			kparalUnsubmitted Not Done Would it make sense to wrap this as well and raise `TaskotronDirectiveError` for IO errors? Or do we want to raise them directly (maybe document them then)?
		264		params['results'] = resultfile.read()
		265
239	check_details = check.import_YAML(params['results'])	266		check_details = check.import_YAML(params['results'])
240	log.debug("YAML output parsed OK.")	267		log.debug("YAML output parsed OK.")
241	except TaskotronValueError as e:	268		except (TaskotronValueError, IOError) as e:
242	raise TaskotronDirectiveError("Failed to load 'results': %s" % e.message)	269		raise TaskotronDirectiveError("Failed to load results: %s" % e.message)
			kparalUnsubmitted Not Done Maybe get rid of the apostrophes in the message, because this is going to be printed even for `file:` now.
			kparalUnsubmitted Not Done IIUIC, this didn't fix the issue unless you also start catching `IOError`.
243		270
244	for detail in check_details:	271		for detail in check_details:
245	if not (detail.item and detail.report_type):	272		if not (detail.item and detail.report_type):
246	raise TaskotronDirectiveError(	273		raise TaskotronDirectiveError(
247	"The resultsdb directive requires "	274		"The resultsdb directive requires "
248	"'item' and 'type' to be present in the YAML data.")	275		"'item' and 'type' to be present in the YAML data.")
249		276
250	conf = config.get_config()	277		conf = config.get_config()
▲ Show 20 Lines • Show All 58 Lines • Show Last 20 Lines

View Options

testing/functest_resultsdb_directive.py

This file was added.

		1		# -- coding: utf-8 --
		2		# Copyright 2009-2016, Red Hat, Inc.
		3		# License: GPL-2.0+ <http://spdx.org/licenses/GPL-2.0+>
		4		# See the LICENSE file for more details on Licensing
		5
		6		import pytest
		7		from dingus import Dingus
		8		import configparser
		9
		10		from libtaskotron.directives import resultsdb_directive
		11
		12		from libtaskotron import check
		13		from libtaskotron import config
		14
		15
		16		class StubConfigParser(object):
		17		def read(self, *args):
		18		pass
		19
		20		def __getitem__(self, key):
		21		return {'url': 'giturl/testname'}
		22
		23
		24		@pytest.mark.usefixtures('setup')
		25		class TestResultsdbDirective():
		26		@pytest.fixture
		27		def setup(self, monkeypatch):
		28		'''Run this before every test invocation'''
		29
		30		self.cd = check.CheckDetail(
		31		item='foo_bar',
		32		report_type=check.ReportType.KOJI_BUILD,
		33		outcome='NEEDS_INSPECTION',
		34		note='foo_bar note',
		35		output=["foo\nbar"],
		36		keyvals={"foo": "moo1", "bar": "moo2"},
		37		)
		38
		39		self.yaml = check.export_YAML(self.cd)
		40
		41		self.ref_input = {'results': self.yaml}
		42		self.ref_arg_data = {
		43		'resultsdb_job_id': 1,
		44		'checkname': 'test_resultsdb_report',
		45		'namespace': 'qa',
		46		'jobid': 'all/123',
		47		'uuid': 'c25237a4-b6b3-11e4-b98a-3c970e018701',
		48		'artifactsdir': '/some/directory/',
		49		'task': '/taskdir',
		50		'item': 'firefox-45.0.2-1.fc23'
		51		}
		52
		53		self.ref_resultdata = {u'id': 1234}
		54
		55		self.ref_jobid = 1234
		56		self.ref_refurl = u'http://example.com/%s' % self.ref_arg_data['checkname']
		57		self.ref_jobdata = {u'end_time': None,
		58		u'href': u'http://127.0.0.1/api/v1.0/jobs/%d' % self.ref_jobid,
		59		u'id': self.ref_jobid,
		60		u'name': self.ref_arg_data['checkname'],
		61		u'ref_url': self.ref_refurl,
		62		u'results': [],
		63		u'results_count': 0,
		64		u'start_time': None,
		65		u'status': u'SCHEDULED'}
		66
		67		self.stub_rdb = Dingus('resultsdb', get_testcase__returns={},
		68		create_job__returns=self.ref_jobdata,
		69		create_result__returns=self.ref_resultdata,
		70		)
		71		self.test_rdb = resultsdb_directive.ResultsdbDirective(self.stub_rdb)
		72
		73		# while it appears useless, this actually sets config in several tests
		74		monkeypatch.setattr(config, '_config', None)
		75		self.conf = config.get_config()
		76		self.conf.report_to_resultsdb = True
		77
		78		monkeypatch.setattr(configparser, 'ConfigParser', StubConfigParser)
		79
		80		def test_param_file(self, tmpdir):
		81		del self.ref_input['results']
		82		input_file = tmpdir.join('results.yaml')
		83		input_file.write(self.yaml)
		84		self.ref_input['file'] = input_file.strpath
		85
		86		self.test_rdb.process(self.ref_input, self.ref_arg_data)
		87
		88		# Given the input data, the resultsdb should be called once, and only
		89		# once, calling "create_result".
		90		# This assert failing either means that more calls were added in the
		91		# source code, or that a bug is present, and "create_result" is
		92		# called multiple times.
		93		# we expect rdb to be called 4 times: create job, update to RUNNING,
		94		# check for testcase, report result and complete job
		95
		96		assert len(self.stub_rdb.calls()) == 5
		97
		98		# Select the first call of "create_result" method.
		99		# This could be written as self.stub_rdb.calls()[0] at the moment, but
		100		# this is more future-proof, and accidental addition of resultsdb
		101		# calls is handled by the previous assert.
		102		call = [call for call in self.stub_rdb.calls() if call[0] == 'create_result'][0]
		103		# Select the keyword arguments of that call
		104		call_data = call[2]
		105
		106		# the log url depends on the arg_data, so construct it here
		107		ref_builder, ref_jobid = self.ref_arg_data['jobid'].split('/')
		108		ref_log_url = '%s/builders/%s/builds/%s/steps/%s/logs/stdio' %\
		109		(self.conf.taskotron_master, ref_builder, ref_jobid,
		110		self.conf.buildbot_task_step)
		111		ref_testcase_name = '%s.%s' % (self.ref_arg_data['namespace'],
		112		self.ref_arg_data['checkname'])
		113
		114		assert call_data['job_id'] == self.ref_jobid
		115		assert call_data['testcase_name'] == ref_testcase_name
		116		assert call_data['outcome'] == self.cd.outcome
		117		assert call_data['summary'] == self.cd.note
		118		assert call_data['log_url'] == ref_log_url
		119		assert call_data['item'] == self.cd.item
		120		assert call_data['type'] == self.cd.report_type
		121
		122		assert 'output' not in call_data.keys()
		123
		124		for key in self.cd.keyvals.keys():
		125		assert call_data[key] == self.cd.keyvals[key]

View Options

testing/test_resultsdb_directive.py

Show First 20 Lines • Show All 172 Lines • ▼ Show 20 Line(s)		136	def test_report(self):
173	assert call_data['item'] == self.cd.item	173		assert call_data['item'] == self.cd.item
174	assert call_data['type'] == self.cd.report_type	174		assert call_data['type'] == self.cd.report_type
175		175
176	assert 'output' not in call_data.keys()	176		assert 'output' not in call_data.keys()
177		177
178	for key in self.cd.keyvals.keys():	178		for key in self.cd.keyvals.keys():
179	assert call_data[key] == self.cd.keyvals[key]	179		assert call_data[key] == self.cd.keyvals[key]
180		180
		181		def test_both_file_and_results(self, monkeypatch):
		182		ref_input = {'results': 'foobar', 'file': 'foo.bar'}
		183		with pytest.raises(TaskotronDirectiveError):
		184		self.test_rdb.process(ref_input, self.ref_arg_data)
			kparalUnsubmitted Not Done This makes me very uneasy. Why we need to mock builtin `open`? That's a big freaking hammer. Would it be better to use pytest's `tmpdir`, as we use it in `functest_yumrepoinfo_directive.py`, to create a fake file with predefined contents (therefore ideally put it into `functest_resultsdb_directive`)?
			jskladanAuthorUnsubmitted Not Done Taken from xunit directive, tbh. I don't really see the problem of mocking `open()`, though, feels even cleaner than creating fake files to me.
			kparalUnsubmitted Not Done I don't like overriding builtins at all, but I won't fight it at this point. However, according to this and this it might be a better idea to patch `open` just for the current module, and not globally in `__builtin__` space.
		185
181	def test_get_artifact_path(self, monkeypatch):	186		def test_get_artifact_path(self, monkeypatch):
182	artifactsdir = '/path/to/artifacts'	187		artifactsdir = '/path/to/artifacts'
183	rel_artifact = 'some/relative/file.log'	188		rel_artifact = 'some/relative/file.log'
184	abs_artifact = os.path.join(artifactsdir, rel_artifact)	189		abs_artifact = os.path.join(artifactsdir, rel_artifact)
185		190
186	monkeypatch.setattr(resultsdb_directive.os.path, 'exists', lambda x: True)	191		monkeypatch.setattr(resultsdb_directive.os.path, 'exists', lambda x: True)
187	assert self.test_rdb.get_artifact_path(artifactsdir, rel_artifact) == rel_artifact	192		assert self.test_rdb.get_artifact_path(artifactsdir, rel_artifact) == rel_artifact
188	assert self.test_rdb.get_artifact_path(artifactsdir, abs_artifact) == rel_artifact	193		assert self.test_rdb.get_artifact_path(artifactsdir, abs_artifact) == rel_artifact
▲ Show 20 Lines • Show All 191 Lines • Show Last 20 Lines

		Path
M		libtaskotron/directives/resultsdb_directive.py (39 lines)
A	M	testing/functest_resultsdb_directive.py (125 lines)
M		testing/test_resultsdb_directive.py (5 lines)

Allow resultsdb_directive to read from file
ClosedPublic
Actions

Details

Diff Detail

Revision Contents

Diff 2414

libtaskotron/directives/resultsdb_directive.py

testing/functest_resultsdb_directive.py

testing/test_resultsdb_directive.py

Allow resultsdb_directive to read from fileClosedPublicActions

Details

Diff Detail

Revision Contents

Diff 2414

libtaskotron/directives/resultsdb_directive.py

testing/functest_resultsdb_directive.py

testing/test_resultsdb_directive.py

Allow resultsdb_directive to read from file
ClosedPublic
Actions