jobtriggers/koji_build_msg.py
8–9	Since you moved most of the code to the class, move this method as well.
18–21	I'd rather see one of these two solutions: rename it to `process`, and call the `_process_XYZ` directly (as hinted in the next comments) move the whole code to `consume` (and call the `_process_XYZ` directly). I don't really see a big gain in having this in a method, that returns a method pointer, so the `consume` can be "one-liner".
32	Y U no `self._process_docker(msg)` instead?
37	ditto
testing/test_koji_build_trigger.py
193–253	Copying the whole code, just so you can change one line (221) is the worst way to do it. I understand that this is probably a proof of concept, but it it stayed, this would be unnecessarily hard to keep in sync. I'd rather see you modify the `_create_msg()` so it takes one more argument, that sets the `owner` to the right value. The `_create_msg()` is called at the beginning of _most_ of the tests, so adding it to the few that remail would not be that big of a deal. When you are at it (and although it's not your code per se), please also change the behaviour of the `_create_msg()` so instead of re-setting the `self._ref_message`, it returns the generated msg, and the tests use the value as input, instead of the `self.ref_message`, that currently gets created "as a side effect". THX.

jobtriggers/koji_build_msg.py
72	Why is this hardcoded and why is it hardcoded to `f24`? Can we not get this information for docker image builds?

jobtriggers/utils.py
51	This should be configurable but that can be added as a feature request
testing/test_koji_build_trigger.py
106	I don't understand why this was needed

jobtriggers/koji_build_msg.py
40	This seems fragile. You didn't publish the whole fedmsg nor linked to it, but isn't there something like `artifact_type: docker` somewhere? And if it isn't, shouldn't we request it? Have you talked to mikem/fedora-releng to verify that all docker builds will be always built under `containerbuild` user? I'm not saying this needs to change right now, but at least a `TODO`/`FIXME` (ideally with a link to a ticket) would be appropriate.

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	2560	c75bb33		Sep 15 2016, 2:33 PM	★	★
Diff 2	2563	c75bb33	- polishing, tests and lint	Sep 16 2016, 9:09 AM	★	★
Diff 3	2565	c75bb33	- code and tests polishing	Sep 16 2016, 11:40 AM	★	★
Diff 4	2566	c75bb33	- changed example	Sep 16 2016, 11:50 AM	★	★
Diff 5	2567	c75bb33	- koji queries for registry url and tag	Sep 16 2016, 2:24 PM	★	★
Diff 6	2624	de639c3	rTRGR5376194b36a5b8dc6025e8e2e7da25c9b313c2b6	Oct 14 2016, 8:30 AM	★	★

Commit	Tree	Parents	Author	Summary	Date
ad43a2b682c4	772bdbedb69e	18aca844cf06	Lukas Brabec	koji queries for registry url and tag	Sep 16 2016, 2:17 PM
18aca844cf06	be863007d3b1	979c667ffa0f	Lukas Brabec	changed example	Sep 16 2016, 11:50 AM
979c667ffa0f	a36f1c85b6a8	c221ab6f8578	Lukas Brabec	code and tests polishing	Sep 16 2016, 11:39 AM
c221ab6f8578	722d393b35a1	1a2c2a0e2461	Lukas Brabec	polishing, tests and lint	Sep 16 2016, 9:08 AM
1a2c2a0e2461	0a42bd7a77eb	c75bb3310daa	Lukas Brabec	support fedmsgs when docker images are built (Show More…)	Sep 16 2016, 8:33 AM

Diff 2624

conf/trigger_rules.yml.example

1	---	1	---
2	- when: {message_type: KojiBuildCompleted}	2	- when: {message_type: KojiBuildPackageCompleted}
3	do:	3	do:
4	- {tasks: [rpmlint, rpmgrill]}	4	- {tasks: [rpmlint, rpmgrill]}
5	- {discover: {repo: 'http://pkgs.fedoraproject.org/git/rpms-checks/${name}.git', branch: "${distgit_branch}"}}	5	- {discover: {repo: 'http://pkgs.fedoraproject.org/git/rpms-checks/${name}.git', branch: "${distgit_branch}"}}
6	- when: {message_type: KojiBuildCompleted, name: docker}	6	- when: {message_type: KojiBuildDockerCompleted}
		7	do:
		8	- {discover: {repo: 'http://pkgs.fedoraproject.org/git/docker-checks/${name}.git', branch: "${distgit_branch}"}}
		9	- when: {message_type: KojiBuildPackageCompleted, name: docker}
7	do:	10	do:
8	- {tasks: [dockerautotest]}	11	- {tasks: [dockerautotest]}
9	- when:	12	- when:
10	message_type: KojiBuildCompleted	13	message_type: KojiBuildPackageCompleted
11	name:	14	name:
12	$in: ${critpath_pkgs}	15	$in: ${critpath_pkgs}
13	$nin: []	16	$nin: []
14	do:	17	do:
15	- {tasks: [abicheck]}	18	- {tasks: [abicheck]}
16	- when: {message_type: KojiTagChanged}	19	- when: {message_type: KojiTagChanged}
17	do:	20	do:
18	- {tasks: [depcheck]}	21	- {tasks: [depcheck]}
19	- when:	22	- when:
20	message_type: KojiTagChanged	23	message_type: KojiTagChanged
21	tag: {$regex: '/.*(?<!testing-pending)$$/'}	24	tag: {$regex: '/.*(?<!testing-pending)$$/'}
22	do:	25	do:
23	- {tasks: [upgradepath]}	26	- {tasks: [upgradepath]}

View Options

jobtriggers/koji_build_msg.py

1	import koji	1		import koji
2	import fedmsg	2		import fedmsg
3	import fedmsg.encoding	3		import fedmsg.encoding
4		4
5	from . import config, utils	5		from . import config, utils
6	from .jobtrigger import JobTrigger	6		from .jobtrigger import JobTrigger
7	from . import exceptions as exc	7		from . import exceptions as exc
8		8
9	ITEM_TYPE = 'koji_build'	9		ITEM_TYPE_PACKAGE = 'koji_build'
			jskladanUnsubmitted Not Done Since you moved most of the code to the class, move this method as well.
10		10		ITEM_TYPE_DOCKER = 'docker_image'
11
12	def get_nvr(msg):
13	release = msg['msg']['release']
14	state = msg['msg']['new']
15	instance = msg['msg']['instance']
16
17	name = msg['msg']['name']
18	version = msg['msg']['version']
19
20	nvr = '-'.join((name, version, release))
21
22	if not instance == 'primary':
23	raise exc.TriggerMsgError('rejecting message not from primary koji instance (%s from %s)' %
24	(nvr, instance))
25
26	if state != koji.BUILD_STATES['COMPLETE']:
27	raise exc.TriggerMsgError('rejecting message for not being a build complete message '
28	'(%s, status %s)' % (nvr, state))
29
30	if 'fc' not in release:
31	raise exc.TriggerMsgError('rejecting message for not containing "fc" in release (%s)' %
32	nvr)
33
34	return name, version, release
35		11
36		12
37	class KojiBuildCompletedJobTrigger(JobTrigger):	13		class KojiBuildCompletedJobTrigger(JobTrigger):
38	topic = "org.fedoraproject.prod.buildsys.build.state.change"	14		topic = "org.fedoraproject.prod.buildsys.build.state.change"
39	jsonify = False	15		jsonify = False
40	config_key = 'taskotron.kojibuildcompletedjobtrigger.enabled'	16		config_key = 'taskotron.kojibuildcompletedjobtrigger.enabled'
41		17
42	def consume(self, message):	18		def get_nvr(self, msg):
43	msg = fedmsg.encoding.loads(message.body)	19		release = msg['msg']['release']
		20		name = msg['msg']['name']
		21		version = msg['msg']['version']
			jskladanUnsubmitted Not Done I'd rather see one of these two solutions: rename it to `process`, and call the `_process_XYZ` directly (as hinted in the next comments) move the whole code to `consume` (and call the `_process_XYZ` directly). I don't really see a big gain in having this in a method, that returns a method pointer, so the `consume` can be "one-liner".
44		22
45	try:	23		return name, version, release
46	name, version, release = get_nvr(msg)	24
47	except exc.TriggerMsgError, e:	25		def process(self, msg):
48	self.log.debug(e)	26		state = msg['msg']['new']
49	return	27		instance = msg['msg']['instance']
		28		owner = msg['msg']['owner']
		29		name, version, release = self.get_nvr(msg)
		30		nvr = '-'.join((name, version, release))
		31
		32		if not instance == 'primary':
			jskladanUnsubmitted Not Done Y U no `self._process_docker(msg)` instead?
		33		raise exc.TriggerMsgError('rejecting message not from primary koji instance '
		34		'(%s from %s)' % (nvr, instance))
		35
		36		if state != koji.BUILD_STATES['COMPLETE']:
		37		raise exc.TriggerMsgError('rejecting message for not being a build complete message '
			jskladanUnsubmitted Not Done ditto
		38		'(%s, status %s)' % (nvr, state))
		39
		40		# FIXME This is probably a temporary approach, could change in future, see D1002#19239
			kparalUnsubmitted Not Done This seems fragile. You didn't publish the whole fedmsg nor linked to it, but isn't there something like `artifact_type: docker` somewhere? And if it isn't, shouldn't we request it? Have you talked to mikem/fedora-releng to verify that all docker builds will be always built under `containerbuild` user? I'm not saying this needs to change right now, but at least a `TODO`/`FIXME` (ideally with a link to a ticket) would be appropriate.
		41		if owner == 'containerbuild':
		42		return self._process_docker(msg)
		43		else:
		44		if 'fc' not in release:
		45		raise exc.TriggerMsgError('rejecting message for not containing "fc" in release '
		46		'(%s)' % nvr)
		47		return self._process_package(msg)
50		48
		49		def _process_package(self, msg):
		50		name, version, release = self.get_nvr(msg)
51	item = '-'.join([name, version, release])	51		item = '-'.join([name, version, release])
52	critpath = utils.parse_yaml_from_file(config.critpath_filepath)	52		critpath = utils.parse_yaml_from_file(config.critpath_filepath)
53	distgit_branch = utils.get_distgit_branch(release)	53		distgit_branch = utils.get_distgit_branch(release)
54	critpath_pkgs = critpath['pkgs'].get(distgit_branch, critpath['pkgs']['master'])	54		critpath_pkgs = critpath['pkgs'].get(distgit_branch, critpath['pkgs']['master'])
55	data = {	55		data = {
56	"_msg": msg,	56		"_msg": msg,
57	"message_type": "KojiBuildCompleted",	57		"message_type": "KojiBuildPackageCompleted",
58	"item": item,	58		"item": item,
59	"item_type": ITEM_TYPE,	59		"item_type": ITEM_TYPE_PACKAGE,
60	"name": name,	60		"name": name,
61	"version": version,	61		"version": version,
62	"release": release,	62		"release": release,
63	"critpath_pkgs": critpath_pkgs,	63		"critpath_pkgs": critpath_pkgs,
64	"distgit_branch": distgit_branch,	64		"distgit_branch": distgit_branch,
65	}	65		}
66	self.do_trigger(data)	66		self.do_trigger(data)
		67
		68		def _process_docker(self, msg):
		69		name, version, release = self.get_nvr(msg)
		70		build_id = msg['msg']['build_id']
		71		item = utils.get_docker_registry_url(build_id)
		72		distgit_branch = utils.get_distgit_branch_docker(build_id)
			tflinkUnsubmitted Not Done Why is this hardcoded and why is it hardcoded to `f24`? Can we not get this information for docker image builds?
		73		critpath_pkgs = [] # FIXME do we need this for docker images?
		74		data = {
		75		"_msg": msg,
		76		"message_type": "KojiBuildDockerCompleted",
		77		"item": item,
		78		"item_type": ITEM_TYPE_DOCKER,
		79		"name": name,
		80		"version": version,
		81		"release": release,
		82		"critpath_pkgs": critpath_pkgs,
		83		"distgit_branch": distgit_branch,
		84		}
		85		self.do_trigger(data)
		86
		87		def consume(self, message):
		88		msg = fedmsg.encoding.loads(message.body)
		89		try:
		90		self.process(msg)
		91		except exc.TriggerMsgError, e:
		92		self.log.debug(e)
		93		return

View Options

jobtriggers/utils.py

1	import os	1		import os
2	import re	2		import re
3	import subprocess	3		import subprocess
4	import csv	4		import csv
5	import datetime	5		import datetime
6	import shutil	6		import shutil
		7		import koji
7		8
8	import yaml	9		import yaml
9		10
10	from jobtriggers import config	11		from jobtriggers import config
11	from jobtriggers import exceptions as exc	12		from jobtriggers import exceptions as exc
12		13
13		14
14	def log_job(logname, item, item_type, task, arch):	15		def log_job(logname, item, item_type, task, arch):
Show All 25 Lines
40	def get_distgit_branch(release):	41		def get_distgit_branch(release):
41	matches = re.findall(r'\.(fc\d{1,2})\.?', release)	42		matches = re.findall(r'\.(fc\d{1,2})\.?', release)
42	if not matches:	43		if not matches:
43	raise exc.TriggerMsgError('Could not parse disttag from %s' % release)	44		raise exc.TriggerMsgError('Could not parse disttag from %s' % release)
44		45
45	return matches[-1].replace('c', '')	46		return matches[-1].replace('c', '')
46		47
47		48
		49		def _get_koji_session():
		50		# for testability reasons...
		51		return koji.ClientSession("http://koji.fedoraproject.org/kojihub")
			tflinkUnsubmitted Not Done This should be configurable but that can be added as a feature request
		52
		53
		54		def get_distgit_branch_docker(build_id):
		55		session = _get_koji_session()
		56		tags = session.listTags(build_id)
		57
		58		for tag in tags:
		59		try:
		60		matches = re.findall(r'^(f\d{1,2})-?.*$', tag['name'])
		61		if matches:
		62		return matches[-1]
		63		except KeyError:
		64		pass
		65
		66		raise exc.TriggerMsgError('Could not get disttag for %d' % build_id)
		67
		68
		69		def get_docker_registry_url(build_id):
		70		session = _get_koji_session()
		71		archives = session.listArchives(build_id, type='image')
		72
		73		for archive in archives:
		74		try:
		75		urls = archive['extra']['docker']['repositories']
		76		for url in urls:
		77		if '@sha' not in url:
		78		return url
		79		except KeyError:
		80		pass
		81
		82		raise exc.TriggerMsgError('Could not get docker registry url from %d' % build_id)
		83
		84
48	def discover_tasks(item_type, repo, branch='master'):	85		def discover_tasks(item_type, repo, branch='master'):
49	dirname = repo.split('/')[-1]	86		dirname = repo.split('/')[-1]
50	if dirname.endswith('.git'):	87		if dirname.endswith('.git'):
51	dirname = dirname[:-len(".git")]	88		dirname = dirname[:-len(".git")]
52		89
53	tasks_dir = os.path.join(config.git_cache_dir, dirname)	90		tasks_dir = os.path.join(config.git_cache_dir, dirname)
54		91
55	# for now, just delete the repo and make a new shallow copy	92		# for now, just delete the repo and make a new shallow copy
Show All 24 Lines

View Options

testing/test_koji_build_trigger.py

1	import koji	1		import koji
2	import pytest	2		import pytest
3	import mock	3		import mock
4	from dingus import Dingus	4		from dingus import Dingus
5	from munch import Munch	5		from munch import Munch
6		6
7	from jobtriggers import koji_build_msg	7		from jobtriggers import koji_build_msg
8	from jobtriggers import utils	8		from jobtriggers import utils
9		9
		10		params = [('someowner', koji_build_msg.ITEM_TYPE_PACKAGE, "KojiBuildPackageCompleted"),
		11		('containerbuild', koji_build_msg.ITEM_TYPE_DOCKER, "KojiBuildDockerCompleted")]
10		12
11	@pytest.mark.usefixtures('prepare')	13		@pytest.mark.usefixtures('prepare')
		14		@pytest.mark.parametrize('buildtype', params)
12	class TestKojiBuildCompletedJobTrigger():	15		class TestKojiBuildCompletedJobTrigger():
13		16
14	@pytest.fixture	17		@pytest.fixture
15	def prepare(self, monkeypatch):	18		def prepare(self, monkeypatch, buildtype):
16	self.ref_instance = 'primary'	19		self.ref_instance = 'primary'
17	self.ref_new = koji.BUILD_STATES['COMPLETE']	20		self.ref_new = koji.BUILD_STATES['COMPLETE']
18	self.ref_name = 'stage'	21		self.ref_name = 'stage'
19	self.ref_version = '4.1.1'	22		self.ref_version = '4.1.1'
20	self.ref_release = '3.fc18'	23		self.ref_release = '3.fc18'
		24		self.ref_owner = buildtype[0]
		25		self.item_type = buildtype[1]
		26		self.message_type = buildtype[2]
21	self.ref_nvr = '%s-%s-%s' % (self.ref_name,	27		self.ref_nvr = '%s-%s-%s' % (self.ref_name,
22	self.ref_version, self.ref_release)	28		self.ref_version, self.ref_release)
23	self.ref_nvr_docker = '%s-%s-%s' % ('docker',	29		self.ref_nvr_docker = '%s-%s-%s' % ('docker',
24	self.ref_version, self.ref_release)	30		self.ref_version, self.ref_release)
25		31
26	self._create_msg(	32		self.ref_message = self._create_msg(self.ref_instance, self.ref_new, self.ref_name,
27	self.ref_instance, self.ref_new, self.ref_name, self.ref_release, self.ref_version)	33		self.ref_release, self.ref_version, self.ref_owner)
28		34
29	self.ref_tasks = ['koji_build_task1', 'koji_build_task2']	35		self.ref_tasks = ['koji_build_task1', 'koji_build_task2']
30	self.ref_tasks_docker = ['docker_build_task']	36		self.ref_tasks_docker = ['docker_build_task']
31	self.ref_validarches = ['i386', 'x86_64']	37		self.ref_validarches = ['i386', 'x86_64']
32		38
33	stub_hub = Munch(config=Munch(get=0))	39		stub_hub = Munch(config=Munch(get=0))
34		40
35	self.helper = koji_build_msg.KojiBuildCompletedJobTrigger(stub_hub)	41		self.helper = koji_build_msg.KojiBuildCompletedJobTrigger(stub_hub)
36	self.helper.runner = Dingus()	42		self.helper.runner = Dingus()
37		43
38	koji_build_msg.config.trigger_rules_template = """---	44		koji_build_msg.config.trigger_rules_template = """---
39	- do:	45		- do:
40	- {discover: {repo: 'http://bogus/firefox.git'}}	46		- {discover: {repo: 'http://bogus/firefox.git'}}
41	- {tasks: %s}	47		- {tasks: %s}
42	when: {message_type: KojiBuildCompleted}	48		when: {message_type: %s}
43	- do:	49		- do:
44	- {tasks: %s}	50		- {tasks: %s}
45	when: {message_type: KojiBuildCompleted, name: docker}	51		when: {message_type: %s, name: docker}
46	""" % (self.ref_tasks, self.ref_tasks_docker)	52		""" % (self.ref_tasks, self.message_type, self.ref_tasks_docker, self.message_type)
47		53
48	koji_build_msg.config.valid_arches = self.ref_validarches	54		koji_build_msg.config.valid_arches = self.ref_validarches
49	koji_build_msg.config.job_logging = False	55		koji_build_msg.config.job_logging = False
50		56
51	mock_discover_tasks = mock.Mock(return_value=[])	57		mock_discover_tasks = mock.Mock(return_value=[])
52	monkeypatch.setattr(utils, 'discover_tasks', mock_discover_tasks)	58		monkeypatch.setattr(utils, 'discover_tasks', mock_discover_tasks)
53		59
54	mock_critpath = {"pkgs": {"f18": [], "master": []}}	60		mock_critpath = {"pkgs": {"f18": [], "master": []}}
55	mock_get_critpath = mock.Mock(return_value=mock_critpath)	61		mock_get_critpath = mock.Mock(return_value=mock_critpath)
56	monkeypatch.setattr(utils, 'parse_yaml_from_file', mock_get_critpath)	62		monkeypatch.setattr(utils, 'parse_yaml_from_file', mock_get_critpath)
57		63
58	def _create_msg(self, ref_instance, ref_new, ref_name, ref_release, ref_version):	64		stub_session = mock.MagicMock()
		65		stub_session.listArchives.return_value = [
		66		{'extra': {'docker': {'repositories': ['image@shaXXX', self.ref_nvr]}}}]
		67		stub_session.listTags.return_value = [{'name': 'ubuntu-docker'}, {'name': 'f24-docker'}]
		68
		69		stub_get_session = mock.MagicMock(return_value=stub_session)
		70		monkeypatch.setattr(utils, '_get_koji_session', stub_get_session)
		71
		72		def _create_msg(self, ref_instance, ref_new, ref_name, ref_release, ref_version, ref_owner):
59	msg = Munch(body='{"i": 1,\	73		msg = Munch(body='{"i": 1,\
60	"msg": {"build_id": 221146,\	74		"msg": {"build_id": 221146,\
		75		"owner": "%s",\
61	"instance": "%s",\	76		"instance": "%s",\
62	"new": %d,\	77		"new": %d,\
63	"name": "%s",\	78		"name": "%s",\
64	"release": "%s",\	79		"release": "%s",\
65	"version": "%s"},\	80		"version": "%s"},\
66	"timestamp": 1359603469.21164,\	81		"timestamp": 1359603469.21164,\
67	"topic": "org.fedoraproject.prod.buildsys.build.state.change",\	82		"topic": "org.fedoraproject.prod.buildsys.build.state.change",\
68	"username": "apache"}' %	83		"username": "apache"}' %
69	(ref_instance, ref_new, ref_name, ref_release, ref_version))	84		(ref_owner, ref_instance, ref_new, ref_name, ref_release, ref_version))
70	self.ref_message = msg	85		return msg
71		86
72	def test_consume(self):	87		def test_consume(self):
73	self.helper.consume(self.ref_message)	88		self.helper.consume(self.ref_message)
74		89
75	runner_calls = self.helper.runner.trigger_job.calls()	90		runner_calls = self.helper.runner.trigger_job.calls()
76		91
77	assert len(runner_calls) == 4	92		assert len(runner_calls) == 4
78		93
79	assert runner_calls[0][1] == (	94		assert runner_calls[0][1] == (self.ref_nvr, self.item_type,
80	self.ref_nvr, koji_build_msg.ITEM_TYPE, self.ref_tasks[0], self.ref_validarches[0])	95		self.ref_tasks[0], self.ref_validarches[0])
81	assert runner_calls[1][1] == (	96		assert runner_calls[1][1] == (self.ref_nvr, self.item_type,
82	self.ref_nvr, koji_build_msg.ITEM_TYPE, self.ref_tasks[0], self.ref_validarches[1])	97		self.ref_tasks[0], self.ref_validarches[1])
83	assert runner_calls[2][1] == (	98		assert runner_calls[2][1] == (self.ref_nvr, self.item_type,
84	self.ref_nvr, koji_build_msg.ITEM_TYPE, self.ref_tasks[1], self.ref_validarches[0])	99		self.ref_tasks[1], self.ref_validarches[0])
85	assert runner_calls[3][1] == (	100		assert runner_calls[3][1] == (self.ref_nvr, self.item_type,
86	self.ref_nvr, koji_build_msg.ITEM_TYPE, self.ref_tasks[1], self.ref_validarches[1])	101		self.ref_tasks[1], self.ref_validarches[1])
87		102
88	def test_consume_docker(self):	103		def test_consume_docker(self):
		104		# skip test when item is docker image
		105		if self.item_type == koji_build_msg.ITEM_TYPE_DOCKER:
		106		return
			tflinkUnsubmitted Not Done I don't understand why this was needed
89	ref_name = 'docker'	107		ref_name = 'docker'
90	self._create_msg(	108		self.ref_message = self._create_msg(self.ref_instance, self.ref_new, ref_name,
91	self.ref_instance, self.ref_new, ref_name, self.ref_release, self.ref_version)	109		self.ref_release, self.ref_version, self.ref_owner)
92		110
93	self.helper.consume(self.ref_message)	111		self.helper.consume(self.ref_message)
94		112
95	runner_calls = self.helper.runner.trigger_job.calls()	113		runner_calls = self.helper.runner.trigger_job.calls()
96		114
97	assert len(runner_calls) == 6	115		assert len(runner_calls) == 6
98		116
99	assert runner_calls[0][1] == (	117		assert runner_calls[0][1] == (self.ref_nvr_docker, self.item_type,
100	self.ref_nvr_docker, koji_build_msg.ITEM_TYPE, self.ref_tasks[0],	118		self.ref_tasks[0], self.ref_validarches[0])
101	self.ref_validarches[0])	119		assert runner_calls[1][1] == (self.ref_nvr_docker, self.item_type,
102	assert runner_calls[1][1] == (	120		self.ref_tasks[0], self.ref_validarches[1])
103	self.ref_nvr_docker, koji_build_msg.ITEM_TYPE, self.ref_tasks[0],	121		assert runner_calls[2][1] == (self.ref_nvr_docker, self.item_type,
104	self.ref_validarches[1])	122		self.ref_tasks[1], self.ref_validarches[0])
105	assert runner_calls[2][1] == (	123		assert runner_calls[3][1] == (self.ref_nvr_docker, self.item_type,
106	self.ref_nvr_docker, koji_build_msg.ITEM_TYPE, self.ref_tasks[1],	124		self.ref_tasks[1], self.ref_validarches[1])
107	self.ref_validarches[0])	125		assert runner_calls[4][1] == (self.ref_nvr_docker, self.item_type,
108	assert runner_calls[3][1] == (	126		self.ref_tasks_docker[0], self.ref_validarches[0])
109	self.ref_nvr_docker, koji_build_msg.ITEM_TYPE, self.ref_tasks[1],	127		assert runner_calls[5][1] == (self.ref_nvr_docker, self.item_type,
110	self.ref_validarches[1])	128		self.ref_tasks_docker[0], self.ref_validarches[1])
111	assert runner_calls[4][1] == (
112	self.ref_nvr_docker, koji_build_msg.ITEM_TYPE, self.ref_tasks_docker[0],
113	self.ref_validarches[0])
114	assert runner_calls[5][1] == (
115	self.ref_nvr_docker, koji_build_msg.ITEM_TYPE, self.ref_tasks_docker[0],
116	self.ref_validarches[1])
117		129
118	def test_consume_distgit(self, monkeypatch):	130		def test_consume_distgit(self, monkeypatch):
119	ref_distgit_task = 'abicheck'	131		ref_distgit_task = 'abicheck'
120	mock_discover_tasks = mock.Mock(return_value=[ref_distgit_task])	132		mock_discover_tasks = mock.Mock(return_value=[ref_distgit_task])
121	monkeypatch.setattr(utils, 'discover_tasks', mock_discover_tasks)	133		monkeypatch.setattr(utils, 'discover_tasks', mock_discover_tasks)
122		134
123	self.helper.consume(self.ref_message)	135		self.helper.consume(self.ref_message)
124		136
125	runner_calls = self.helper.runner.trigger_job.calls()	137		runner_calls = self.helper.runner.trigger_job.calls()
126		138
127	assert len(runner_calls) == 6	139		assert len(runner_calls) == 6
128		140
129	assert runner_calls[0][1] == (	141		assert runner_calls[0][1] == (self.ref_nvr, self.item_type,
130	self.ref_nvr, koji_build_msg.ITEM_TYPE, ref_distgit_task, self.ref_validarches[0])	142		ref_distgit_task, self.ref_validarches[0])
131	assert runner_calls[1][1] == (	143		assert runner_calls[1][1] == (self.ref_nvr, self.item_type,
132	self.ref_nvr, koji_build_msg.ITEM_TYPE, ref_distgit_task, self.ref_validarches[1])	144		ref_distgit_task, self.ref_validarches[1])
133	assert runner_calls[2][1] == (	145		assert runner_calls[2][1] == (self.ref_nvr, self.item_type,
134	self.ref_nvr, koji_build_msg.ITEM_TYPE, self.ref_tasks[0], self.ref_validarches[0])	146		self.ref_tasks[0], self.ref_validarches[0])
135	assert runner_calls[3][1] == (	147		assert runner_calls[3][1] == (self.ref_nvr, self.item_type,
136	self.ref_nvr, koji_build_msg.ITEM_TYPE, self.ref_tasks[0], self.ref_validarches[1])	148		self.ref_tasks[0], self.ref_validarches[1])
137	assert runner_calls[4][1] == (	149		assert runner_calls[4][1] == (self.ref_nvr, self.item_type,
138	self.ref_nvr, koji_build_msg.ITEM_TYPE, self.ref_tasks[1], self.ref_validarches[0])	150		self.ref_tasks[1], self.ref_validarches[0])
139	assert runner_calls[5][1] == (	151		assert runner_calls[5][1] == (self.ref_nvr, self.item_type,
140	self.ref_nvr, koji_build_msg.ITEM_TYPE, self.ref_tasks[1], self.ref_validarches[1])	152		self.ref_tasks[1], self.ref_validarches[1])
141		153
142	def test_consume_not_primary_instance(self):	154		def test_consume_not_primary_instance(self):
143	ref_instance = 'ppc'	155		ref_instance = 'ppc'
144	self._create_msg(	156		self.ref_message = self._create_msg(ref_instance, self.ref_new, self.ref_name,
145	ref_instance, self.ref_new, self.ref_name, self.ref_release, self.ref_version)	157		self.ref_release, self.ref_version, self.ref_owner)
146		158
147	self.helper.consume(self.ref_message)	159		self.helper.consume(self.ref_message)
148		160
149	runner_calls = self.helper.runner.trigger_job.calls()	161		runner_calls = self.helper.runner.trigger_job.calls()
150		162
151	assert runner_calls == []	163		assert runner_calls == []
152		164
153	def test_consume_notcomplete_state(self):	165		def test_consume_notcomplete_state(self):
154	ref_new = koji.BUILD_STATES['BUILDING']	166		ref_new = koji.BUILD_STATES['BUILDING']
155	self._create_msg(	167		self.ref_message = self._create_msg(self.ref_instance, ref_new, self.ref_name,
156	self.ref_instance, ref_new, self.ref_name, self.ref_release, self.ref_version)	168		self.ref_release, self.ref_version, self.ref_owner)
157		169
158	self.helper.consume(self.ref_message)	170		self.helper.consume(self.ref_message)
159		171
160	runner_calls = self.helper.runner.trigger_job.calls()	172		runner_calls = self.helper.runner.trigger_job.calls()
161		173
162	assert runner_calls == []	174		assert runner_calls == []
163		175
164	def test_consume_blacklisted_release(self):	176		def test_consume_blacklisted_release(self):
		177		# skip test when item is docker image
		178		if self.item_type == koji_build_msg.ITEM_TYPE_DOCKER:
		179		return
		180
165	ref_release = '3.el5.1'	181		ref_release = '3.el5.1'
166	self._create_msg(	182		self.ref_message = self._create_msg(self.ref_instance, self.ref_new, self.ref_name,
167	self.ref_instance, self.ref_new, self.ref_name, ref_release, self.ref_version)	183		ref_release, self.ref_version, self.ref_owner)
168		184
169	self.helper.consume(self.ref_message)	185		self.helper.consume(self.ref_message)
170		186
171	runner_calls = self.helper.runner.trigger_job.calls()	187		runner_calls = self.helper.runner.trigger_job.calls()
172		188
173	assert runner_calls == []	189		assert runner_calls == []
			jskladanUnsubmitted Not Done Copying the whole code, just so you can change one line (221) is the worst way to do it. I understand that this is probably a proof of concept, but it it stayed, this would be unnecessarily hard to keep in sync. I'd rather see you modify the `_create_msg()` so it takes one more argument, that sets the `owner` to the right value. The `_create_msg()` is called at the beginning of _most_ of the tests, so adding it to the few that remail would not be that big of a deal. When you are at it (and although it's not your code per se), please also change the behaviour of the `_create_msg()` so instead of re-setting the `self._ref_message`, it returns the generated msg, and the tests use the value as input, instead of the `self.ref_message`, that currently gets created "as a side effect". THX.

			Path	Packages
M			conf/trigger_rules.yml.example (9 lines)
M			jobtriggers/koji_build_msg.py (97 lines)
M			jobtriggers/utils.py (37 lines)
M			testing/test_koji_build_trigger.py (126 lines)

support fedmsgs when docker images are built
ClosedPublic
Actions

Details

Diff Detail

Revision Contents

Diff 2624

conf/trigger_rules.yml.example

jobtriggers/koji_build_msg.py

jobtriggers/utils.py

testing/test_koji_build_trigger.py

support fedmsgs when docker images are builtClosedPublicActions

Details

Diff Detail

Revision Contents

Diff 2624

conf/trigger_rules.yml.example

jobtriggers/koji_build_msg.py

jobtriggers/utils.py

testing/test_koji_build_trigger.py

support fedmsgs when docker images are built
ClosedPublic
Actions