PR#40: Adding support for standard test roles. Fixes #39 - taskotron/taskotron-trigger

taskotron / taskotron-trigger

#40 Adding support for standard test roles. Fixes #39

Closed 6 years ago by kparal. Opened 6 years ago by tflink.

taskotron/ tflink/taskotron-trigger feature/39-support-standard-roles into feature/ansiblize

removing debug pprint that shouldn't be there

Tim Flink • 6 years ago

400aa06

Adding support for standard test roles. Fixes #39

Tim Flink • 6 years ago

0fa102f

jobtriggers/jobtrigger.py

file modified

		`@@ -71,6 +71,12 @@`
		`repo = discover['repo']`
		`branch = discover.get('branch', 'master').strip() or 'master'`
		`fallback = discover.get('fallback_branch', '').strip()`
		`+`
		`+ # make sure that the repo exists before we try to clone it`
		`+ if not utils.git_repo_exists(repo):`
		`+ self.log.info("Repo {} does not exist".format(repo))`
		`+ continue`
		`+`
		`repo_dir, branch = utils.clone_repo(repo, branch, fallback)`

		`# append base_dir, if specified`

jobtriggers/utils.py

file modified

+21 -9

		`@@ -5,6 +5,7 @@`
		`import datetime`
		`import shutil`
		`import koji`
		`+ import urllib2`

		`import yaml`

		`@@ -29,14 +30,10 @@`


		`def _should_schedule(formula_file, item_type):`
		`- formula = parse_yaml_from_file(formula_file)`
		`- try:`
		`- run = formula['scheduling']['run']`
		`- except KeyError:`
		`- run = True`
		`-`
		`- return run and item_type in formula['input']['args']`
		`+ # it's not clear whether we want to keep this feature as we move away from`
		`+ # custom YAML formulae. Leaving this in as a stub for now.`

		`+ return True`

		`def get_distgit_branch(release):`
		`matches = re.findall(r'\.(fc\d{1,2})\.?', release)`
		`@@ -119,8 +116,8 @@`
		`directory = directory.strip().rstrip('/')`

		`for dirpath, dirs, files in os.walk(directory):`
		`- if 'runtask.yml' in files:`
		`- formula = os.path.join(dirpath, 'runtask.yml')`
		`+ if 'tests.yml' in files:`
		`+ formula = os.path.join(dirpath, 'tests.yml')`
jskladan commented 6 years ago This feels like a good thing to put to a config file.
		`# remove the base directory path`
		`formulae.append(formula[len(directory)+1:])`
		`if not recursive:`
		`@@ -140,3 +137,18 @@`
		`tasks = [f for f in formulae if _should_schedule(os.path.join(directory, f), item_type)]`

		`return tasks`
		`+`
		`+ def git_repo_exists(repo):`
		`+ """Attempts to open a connection to the repo parameter. Returns False if`
		`+ there were issues in connecting, otherwise returns True"""`
		`+ try:`
		`+ ret = urllib2.urlopen(repo)`
		`+ except urllib2.URLError:`
		`+ return False`
		`+`
		`+ if ret.code != 200:`
		`+ return False`
		`+`
		`+ return True`
		`+`
		`+`

testing/test_jobtrigger.py

file modified

+25

		`@@ -51,6 +51,11 @@`

		`jobtrigger.config.valid_arches = self.ref_default_arches`

		`+ # short-circuit the logic to check if a repo exists`
		`+ stub_repoexist = mock.Mock(return_value=True)`
		`+ monkeypatch.setattr(utils, 'git_repo_exists', stub_repoexist)`
		`+`
		`+`
		`def test_load_rules(self):`
		`template = """$in: [${foo}, ${in}]"""`
		`data = {"foo": "bar", "in": "out"}`
		`@@ -168,3 +173,23 @@`
		`runner_calls = self.helper.runner.trigger_job.calls()`

		`assert len(runner_calls) == 0`
		`+`
		`+ def test_discover_repo_exists(self, monkeypatch):`
		`+ """Make sure that when a repo does not exist, a clone is not attempted"""`
		`+`
		`+ stub_clone_repo = mock.Mock(return_value=('/tmp/path_to_repo', 'master'))`
		`+ monkeypatch.setattr(utils, 'clone_repo', stub_clone_repo)`
		`+`
		`+ stub_repo_exist = mock.Mock(return_value=False)`
		`+ monkeypatch.setattr(utils, 'git_repo_exists', stub_repo_exist)`
		`+`
		`+ mock__load_rules = mock.Mock(return_value=self.ref_rules)`
		`+ monkeypatch.setattr(self.helper, '_load_rules', mock__load_rules)`
		`+`
		`+ rule = {'do': [{'discover': {'repo': 'http://bogus.repo/${name}.git'}}],`
		`+ 'when': {'message_type': 'DiscoverTasks'}}`
		`+`
		`+ self.helper._tasks_from_rule(rule, self.ref_message_data)`
		`+`
		`+ stub_repo_exist.assert_called()`
		`+ stub_clone_repo.assert_not_called()`

testing/test_koji_build_trigger.py

file modified

		`@@ -74,6 +74,10 @@`
		`stub_get_session = mock.MagicMock(return_value=stub_session)`
		`monkeypatch.setattr(utils, '_get_koji_session', stub_get_session)`

		`+ # short-circuit the logic to check if a repo exists`
		`+ stub_repoexist = mock.Mock(return_value=True)`
		`+ monkeypatch.setattr(utils, 'git_repo_exists', stub_repoexist)`
		`+`
		`def _create_msg(self, ref_instance, ref_new, ref_name, ref_release, ref_version, ref_owner):`
		`msg = Munch(body='{"i": 1,\`
		`"msg": {"build_id": 221146,\`

testing/test_utils.py

file modified

+4 -40

		`@@ -46,42 +46,6 @@`
		`with mock.patch('__builtin__.open', mocked_open):`
		`assert utils._should_schedule('mocked_filename', 'koji_build')`

		`- def test_check__should_schedule_false(self):`
		`- file_data = '''`
		`- scheduling:`
		`- run: False`
		`-`
		`- input:`
		`- args:`
		`- - koji_build`
		`- '''`
		`- mocked_open = mock.mock_open(read_data=file_data)`
		`-`
		`- with mock.patch('__builtin__.open', mocked_open):`
		`- assert not utils._should_schedule('mocked_filename', 'koji_build')`
		`-`
		`- def test_check__should_schedule_default(self):`
		`- file_data = '''`
		`- input:`
		`- args:`
		`- - koji_build`
		`- '''`
		`- mocked_open = mock.mock_open(read_data=file_data)`
		`-`
		`- with mock.patch('__builtin__.open', mocked_open):`
		`- assert utils._should_schedule('mocked_filename', 'koji_build')`
		`-`
		`- def test_check__should_schedule_false_item_type(self):`
		`- file_data = '''`
		`- input:`
		`- args:`
		`- - koji_tag`
		`- '''`
		`- mocked_open = mock.mock_open(read_data=file_data)`
		`-`
		`- with mock.patch('__builtin__.open', mocked_open):`
		`- assert not utils._should_schedule('mocked_filename', 'koji_build')`
		`-`
		`def test_clone_repo(self, monkeypatch):`
		`mock_path_exists = mock.Mock(return_value=False)`
		`mock_check_output = mock.Mock(return_value='')`
		`@@ -131,19 +95,19 @@`

		`def test__discover_formulae(self, monkeypatch):`
		`def mock_os_walk(directory):`
		`- yield (directory, ['task_1', 'nontask'], ['runtask.yml'])`
		`+ yield (directory, ['task_1', 'nontask'], ['tests.yml'])`
		`yield ('%s/nontask' % directory, [], [])`
		`- yield ('%s/task_1' % directory, [], ['runtask.yml'])`
		`+ yield ('%s/task_1' % directory, [], ['tests.yml'])`

		`monkeypatch.setattr(os, 'walk', mock_os_walk)`

		`formulae = utils._discover_formulae(' /path/to/ ', recursive=True)`
		`assert len(formulae) == 2`
		`- assert formulae == ['runtask.yml', 'task_1/runtask.yml']`
		`+ assert formulae == ['tests.yml', 'task_1/tests.yml']`

		`formulae = utils._discover_formulae('/path/to')`
		`assert len(formulae) == 1`
		`- assert formulae == ['runtask.yml']`
		`+ assert formulae == ['tests.yml']`

		`def test_discover_tasks(self, monkeypatch):`
		`mock__discover_formulae = mock.Mock(return_value=['task1.yml', 'task2.yml'])`

tflink commented 6 years ago

The changes in this commit are:
- change the file we discover from 'runtask.yml' to 'tests.yml'
- check for existance of a repo before attempting to clone it
- removal of checks and tests for the scheduling part of old forumulae