From cfe231f618ac0a0b97adc94e68cb061f7f0fd9bb Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 13 Feb 2017 13:22:59 -0500 Subject: [PATCH 01/10] Add unit testing of custom trigger handler --- buildtrigger/customhandler.py | 10 ++--- buildtrigger/test/test_customhandler.py | 51 +++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 5 deletions(-) create mode 100644 buildtrigger/test/test_customhandler.py diff --git a/buildtrigger/customhandler.py b/buildtrigger/customhandler.py index 7541995a5..193445ee2 100644 --- a/buildtrigger/customhandler.py +++ b/buildtrigger/customhandler.py @@ -16,9 +16,6 @@ from buildtrigger.bitbuckethandler import (BITBUCKET_WEBHOOK_PAYLOAD_SCHEMA as b from buildtrigger.githubhandler import (GITHUB_WEBHOOK_PAYLOAD_SCHEMA as gh_schema, get_transformed_webhook_payload as gh_payload) -from buildtrigger.bitbuckethandler import (BITBUCKET_WEBHOOK_PAYLOAD_SCHEMA as bb_schema, - get_transformed_webhook_payload as bb_payload) - from buildtrigger.gitlabhandler import (GITLAB_WEBHOOK_PAYLOAD_SCHEMA as gl_schema, get_transformed_webhook_payload as gl_payload) @@ -162,7 +159,7 @@ class CustomBuildTrigger(BuildTriggerHandler): def handle_trigger_request(self, request): payload = request.data if not payload: - raise InvalidPayloadException() + raise InvalidPayloadException('Missing expected payload') logger.debug('Payload %s', payload) @@ -186,7 +183,10 @@ class CustomBuildTrigger(BuildTriggerHandler): 'git_url': config['build_source'], } - return self.prepare_build(metadata, is_manual=True) + try: + return self.prepare_build(metadata, is_manual=True) + except ValidationError as ve: + raise TriggerStartException(ve.message) def activate(self, standard_webhook_url): config = self.config diff --git a/buildtrigger/test/test_customhandler.py b/buildtrigger/test/test_customhandler.py new file mode 100644 index 000000000..6d05cb2b9 --- /dev/null +++ b/buildtrigger/test/test_customhandler.py @@ -0,0 +1,51 @@ +import pytest + +from buildtrigger.customhandler import CustomBuildTrigger +from buildtrigger.triggerutil import (InvalidPayloadException, SkipRequestException, + TriggerStartException) +from endpoints.building import PreparedBuild +from util.morecollections import AttrDict + +@pytest.mark.parametrize('payload, expected_error, expected_message', [ + ('', InvalidPayloadException, 'Missing expected payload'), + ('{}', InvalidPayloadException, "'commit' is a required property"), + + ('{"commit": "foo", "ref": "bar", "default_branch": "baz"}', + InvalidPayloadException, "u'foo' does not match '^([A-Fa-f0-9]{7,})$'"), + + ('{"commit": "11d6fbc", "ref": "refs/heads/something", "default_branch": "baz"}', None, None), + ('''{ + "commit": "11d6fbc", + "ref": "refs/heads/something", + "default_branch": "baz", + "commit_info": { + "message": "[skip build]", + "url": "http://foo.bar", + "date": "NOW" + } + }''', SkipRequestException, ''), +]) +def test_handle_trigger_request(payload, expected_error, expected_message): + trigger = CustomBuildTrigger(None, {'build_source': 'foo'}) + request = AttrDict(dict(data=payload)) + + if expected_error is not None: + with pytest.raises(expected_error) as ipe: + trigger.handle_trigger_request(request) + assert ipe.value.message == expected_message + else: + assert isinstance(trigger.handle_trigger_request(request), PreparedBuild) + +@pytest.mark.parametrize('run_parameters, expected_error, expected_message', [ + ({}, TriggerStartException, 'missing required parameter'), + ({'commit_sha': 'foo'}, TriggerStartException, "'foo' does not match '^([A-Fa-f0-9]{7,})$'"), + ({'commit_sha': '11d6fbc'}, None, None), +]) +def test_manual_start(run_parameters, expected_error, expected_message): + trigger = CustomBuildTrigger(None, {'build_source': 'foo'}) + if expected_error is not None: + with pytest.raises(expected_error) as ipe: + trigger.manual_start(run_parameters) + assert ipe.value.message == expected_message + else: + assert isinstance(trigger.manual_start(run_parameters), PreparedBuild) From c4f873ae969faf002f2b6a0f2f00756de6d0c643 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 13 Feb 2017 13:23:07 -0500 Subject: [PATCH 02/10] Add unit testing of github trigger handler --- buildtrigger/githubhandler.py | 44 ++- buildtrigger/test/test_githubhandler.py | 361 ++++++++++++++++++++++++ 2 files changed, 392 insertions(+), 13 deletions(-) create mode 100644 buildtrigger/test/test_githubhandler.py diff --git a/buildtrigger/githubhandler.py b/buildtrigger/githubhandler.py index 99f90bdd5..a51526507 100644 --- a/buildtrigger/githubhandler.py +++ b/buildtrigger/githubhandler.py @@ -261,13 +261,14 @@ class GithubBuildTrigger(BuildTriggerHandler): raise TriggerDeactivationException(msg) # Remove the webhook. - try: - hook = repo.get_hook(config['hook_id']) - hook.delete() - except GithubException as ghe: - default_msg = 'Unable to remove hook: %s' % config['hook_id'] - msg = GithubBuildTrigger._get_error_message(ghe, default_msg) - raise TriggerDeactivationException(msg) + if 'hook_id' in config: + try: + hook = repo.get_hook(config['hook_id']) + hook.delete() + except GithubException as ghe: + default_msg = 'Unable to remove hook: %s' % config['hook_id'] + msg = GithubBuildTrigger._get_error_message(ghe, default_msg) + raise TriggerDeactivationException(msg) config.pop('hook_id', None) self.config = config @@ -315,12 +316,14 @@ class GithubBuildTrigger(BuildTriggerHandler): gh_client = self._get_client() usr = gh_client.get_user() - if namespace == usr.login: return [repo_view(repo) for repo in usr.get_repos() if repo.owner.login == namespace] - org = gh_client.get_organization(namespace) - if org is None: + try: + org = gh_client.get_organization(namespace) + if org is None: + return [] + except GithubException: return [] return [repo_view(repo) for repo in org.get_repos(type='member')] @@ -479,8 +482,11 @@ class GithubBuildTrigger(BuildTriggerHandler): raise TriggerStartException(msg) def get_branch_sha(branch_name): - branch = repo.get_branch(branch_name) - return branch.commit.sha + try: + branch = repo.get_branch(branch_name) + return branch.commit.sha + except GithubException: + raise TriggerStartException('Could not find branch in repository') def get_tag_sha(tag_name): tags = {tag.name: tag for tag in repo.get_tags()} @@ -515,9 +521,21 @@ class GithubBuildTrigger(BuildTriggerHandler): # This is for GitHub's probing/testing. if 'zen' in payload: - raise ValidationRequestException() + raise SkipRequestException() # Lookup the default branch for the repository. + if 'repository' not in payload: + raise ValidationRequestException("Missing 'repository' on request") + + if 'owner' not in payload['repository']: + raise ValidationRequestException("Missing 'owner' on repository") + + if 'name' not in payload['repository']['owner']: + raise ValidationRequestException("Missing owner 'name' on repository") + + if 'name' not in payload['repository']: + raise ValidationRequestException("Missing 'name' on repository") + default_branch = None lookup_user = None try: diff --git a/buildtrigger/test/test_githubhandler.py b/buildtrigger/test/test_githubhandler.py new file mode 100644 index 000000000..3d6555f81 --- /dev/null +++ b/buildtrigger/test/test_githubhandler.py @@ -0,0 +1,361 @@ +import json +import pytest + +from github import GithubException +from mock import Mock +from datetime import datetime + +from buildtrigger.githubhandler import GithubBuildTrigger +from buildtrigger.triggerutil import (InvalidPayloadException, SkipRequestException, + TriggerStartException, ValidationRequestException) +from endpoints.building import PreparedBuild +from util.morecollections import AttrDict + +def get_mock_github(): + def get_commit_mock(commit_sha): + if commit_sha == 'aaaaaaa': + commit_mock = Mock() + commit_mock.sha = commit_sha + commit_mock.html_url = 'http://url/to/commit' + commit_mock.last_modified = 'now' + + commit_mock.commit = Mock() + commit_mock.commit.message = 'some cool message' + + commit_mock.committer = Mock() + commit_mock.committer.login = 'someuser' + commit_mock.committer.avatar_url = 'avatarurl' + commit_mock.committer.html_url = 'htmlurl' + + commit_mock.author = Mock() + commit_mock.author.login = 'someuser' + commit_mock.author.avatar_url = 'avatarurl' + commit_mock.author.html_url = 'htmlurl' + return commit_mock + + raise GithubException(None, None) + + def get_branch_mock(branch_name): + if branch_name == 'master': + branch_mock = Mock() + branch_mock.commit = Mock() + branch_mock.commit.sha = 'aaaaaaa' + return branch_mock + + raise GithubException(None, None) + + def get_repo_mock(namespace, name): + repo_mock = Mock() + repo_mock.owner = Mock() + repo_mock.owner.login = namespace + + repo_mock.full_name = '%s/%s' % (namespace, name) + repo_mock.name = name + repo_mock.description = 'some %s repo' % (name) + repo_mock.pushed_at = datetime.utcfromtimestamp(0) + repo_mock.html_url = 'http://some/url' + repo_mock.private = name == 'somerepo' + repo_mock.permissions = Mock() + repo_mock.permissions.admin = namespace == 'knownuser' + return repo_mock + + def get_user_repos_mock(): + return [get_repo_mock('knownuser', 'somerepo')] + + def get_org_repos_mock(type='all'): + return [get_repo_mock('someorg', 'somerepo'), get_repo_mock('someorg', 'anotherrepo')] + + def get_orgs_mock(): + return [get_org_mock('someorg')] + + def get_user_mock(username='knownuser'): + if username == 'knownuser': + user_mock = Mock() + user_mock.name = username + user_mock.plan = Mock() + user_mock.plan.private_repos = 1 + user_mock.login = username + user_mock.html_url = 'htmlurl' + user_mock.avatar_url = 'avatarurl' + user_mock.get_repos = Mock(side_effect=get_user_repos_mock) + user_mock.get_orgs = Mock(side_effect=get_orgs_mock) + return user_mock + + raise GithubException(None, None) + + def get_org_mock(namespace): + if namespace == 'someorg': + org_mock = Mock() + org_mock.get_repos = Mock(side_effect=get_org_repos_mock) + org_mock.login = namespace + org_mock.html_url = 'htmlurl' + org_mock.avatar_url = 'avatarurl' + org_mock.name = namespace + org_mock.plan = Mock() + org_mock.plan.private_repos = 2 + return org_mock + + raise GithubException(None, None) + + def get_tags_mock(): + sometag = Mock() + sometag.name = 'sometag' + sometag.commit = get_commit_mock('aaaaaaa') + + someothertag = Mock() + someothertag.name = 'someothertag' + someothertag.commit = get_commit_mock('aaaaaaa') + return [sometag, someothertag] + + def get_branches_mock(): + master = Mock() + master.name = 'master' + master.commit = get_commit_mock('aaaaaaa') + + otherbranch = Mock() + otherbranch.name = 'otherbranch' + otherbranch.commit = get_commit_mock('aaaaaaa') + return [master, otherbranch] + + def get_file_contents_mock(filepath): + if filepath == '/Dockerfile': + m = Mock() + m.content = 'hello world' + return m + + if filepath == 'somesubdir/Dockerfile': + m = Mock() + m.content = 'hi universe' + return m + + raise GithubException(None, None) + + def get_git_tree_mock(commit_sha, recursive=False): + first_file = Mock() + first_file.type = 'blob' + first_file.path = 'Dockerfile' + + second_file = Mock() + second_file.type = 'other' + second_file.path = '/some/Dockerfile' + + third_file = Mock() + third_file.type = 'blob' + third_file.path = 'somesubdir/Dockerfile' + + t = Mock() + + if commit_sha == 'aaaaaaa': + t.tree = [ + first_file, second_file, third_file, + ] + else: + t.tree = [] + + return t + + repo_mock = Mock() + repo_mock.default_branch = 'master' + repo_mock.ssh_url = 'ssh_url' + + repo_mock.get_branch = Mock(side_effect=get_branch_mock) + repo_mock.get_tags = Mock(side_effect=get_tags_mock) + repo_mock.get_branches = Mock(side_effect=get_branches_mock) + repo_mock.get_commit = Mock(side_effect=get_commit_mock) + repo_mock.get_file_contents = Mock(side_effect=get_file_contents_mock) + repo_mock.get_git_tree = Mock(side_effect=get_git_tree_mock) + + gh_mock = Mock() + gh_mock.get_repo = Mock(return_value=repo_mock) + gh_mock.get_user = Mock(side_effect=get_user_mock) + gh_mock.get_organization = Mock(side_effect=get_org_mock) + return gh_mock + +@pytest.fixture +def github_trigger(): + return _get_github_trigger() + +def _get_github_trigger(subdir=''): + trigger_obj = AttrDict(dict(auth_token='foobar', id='sometrigger')) + trigger = GithubBuildTrigger(trigger_obj, {'build_source': 'foo', 'subdir': subdir}) + trigger._get_client = get_mock_github + return trigger + +@pytest.mark.parametrize('payload, expected_error, expected_message', [ + ('{"zen": true}', SkipRequestException, ""), + + ('{}', ValidationRequestException, "Missing 'repository' on request"), + ('{"repository": "foo"}', ValidationRequestException, "Missing 'owner' on repository"), + + # Valid payload: + ('''{ + "repository": { + "owner": { + "name": "someguy" + }, + "name": "somerepo", + "ssh_url": "someurl" + }, + "ref": "refs/tags/foo", + "head_commit": { + "id": "11d6fbc", + "url": "http://some/url", + "message": "some message", + "timestamp": "NOW" + } + }''', None, None), + + # Skip message: + ('''{ + "repository": { + "owner": { + "name": "someguy" + }, + "name": "somerepo", + "ssh_url": "someurl" + }, + "ref": "refs/tags/foo", + "head_commit": { + "id": "11d6fbc", + "url": "http://some/url", + "message": "[skip build]", + "timestamp": "NOW" + } + }''', SkipRequestException, ''), +]) +def test_handle_trigger_request(github_trigger, payload, expected_error, expected_message): + def get_payload(): + return json.loads(payload) + + request = AttrDict(dict(get_json=get_payload)) + + if expected_error is not None: + with pytest.raises(expected_error) as ipe: + github_trigger.handle_trigger_request(request) + assert ipe.value.message == expected_message + else: + assert isinstance(github_trigger.handle_trigger_request(request), PreparedBuild) + + +@pytest.mark.parametrize('run_parameters, expected_error, expected_message', [ + # No branch or tag specified: use the commit of the default branch. + ({}, None, None), + + # Invalid branch. + ({'refs': {'kind': 'branch', 'name': 'invalid'}}, TriggerStartException, + 'Could not find branch in repository'), + + # Invalid tag. + ({'refs': {'kind': 'tag', 'name': 'invalid'}}, TriggerStartException, + 'Could not find tag in repository'), + + # Valid branch. + ({'refs': {'kind': 'branch', 'name': 'master'}}, None, None), + + # Valid tag. + ({'refs': {'kind': 'tag', 'name': 'sometag'}}, None, None), +]) +def test_manual_start(run_parameters, expected_error, expected_message, github_trigger): + if expected_error is not None: + with pytest.raises(expected_error) as ipe: + github_trigger.manual_start(run_parameters) + assert ipe.value.message == expected_message + else: + assert isinstance(github_trigger.manual_start(run_parameters), PreparedBuild) + + +@pytest.mark.parametrize('username, expected_response', [ + ('unknownuser', None), + ('knownuser', {'html_url': 'htmlurl', 'avatar_url': 'avatarurl'}), +]) +def test_lookup_user(username, expected_response, github_trigger): + assert github_trigger.lookup_user(username) == expected_response + + +@pytest.mark.parametrize('name, expected', [ + ('refs', [ + {'kind': 'branch', 'name': 'master'}, + {'kind': 'branch', 'name': 'otherbranch'}, + {'kind': 'tag', 'name': 'sometag'}, + {'kind': 'tag', 'name': 'someothertag'}, + ]), + ('tag_name', ['sometag', 'someothertag']), + ('branch_name', ['master', 'otherbranch']), + ('invalid', None) +]) +def test_list_field_values(name, expected, github_trigger): + assert github_trigger.list_field_values(name) == expected + + +@pytest.mark.parametrize('subdir, contents', [ + ('', 'hello world'), + ('somesubdir', 'hi universe'), + ('unknownpath', None), +]) +def test_load_dockerfile_contents(subdir, contents): + trigger = _get_github_trigger(subdir) + assert trigger.load_dockerfile_contents() == contents + + +def test_list_build_subdirs(github_trigger): + assert github_trigger.list_build_subdirs() == ['', 'somesubdir'] + + +def test_list_build_source_namespaces(github_trigger): + namespaces_expected = [ + { + 'personal': True, + 'score': 1, + 'avatar_url': 'avatarurl', + 'id': 'knownuser', + 'title': 'knownuser' + }, + { + 'score': 2, + 'title': 'someorg', + 'personal': False, + 'url': 'htmlurl', + 'avatar_url': 'avatarurl', + 'id': 'someorg' + } + ] + assert github_trigger.list_build_source_namespaces() == namespaces_expected + + +@pytest.mark.parametrize('namespace, expected', [ + ('', []), + ('unknown', []), + + ('knownuser', [ + { + 'last_updated': 0, 'name': 'somerepo', 'url': 'http://some/url', 'private': True, + 'full_name': 'knownuser/somerepo', 'has_admin_permissions': True, + 'description': 'some somerepo repo' + }]), + + ('someorg', [ + { + 'last_updated': 0, 'name': 'somerepo', 'url': 'http://some/url', + 'private': True, 'full_name': 'someorg/somerepo', 'has_admin_permissions': False, + 'description': 'some somerepo repo' + }, + { + 'last_updated': 0, 'name': 'anotherrepo', 'url': 'http://some/url', + 'private': False, 'full_name': 'someorg/anotherrepo', 'has_admin_permissions': False, + 'description': 'some anotherrepo repo' + }]), +]) +def test_list_build_sources_for_namespace(namespace, expected, github_trigger): + # TODO: schema validation on the resulting namespaces. + assert github_trigger.list_build_sources_for_namespace(namespace) == expected + + +def test_activate(github_trigger): + config, private_key = github_trigger.activate('http://some/url') + assert 'deploy_key_id' in config + assert 'hook_id' in config + assert 'private_key' in private_key + + +def test_deactivate(github_trigger): + github_trigger.deactivate() From ba301b401b723d192766ea000d180f9c3329add5 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 13 Feb 2017 13:50:14 -0500 Subject: [PATCH 03/10] Break common git hosts tests into their own suite --- buildtrigger/test/__init__.py | 0 buildtrigger/test/githubmock.py | 173 ++++++++++++ buildtrigger/test/test_bitbuckethandler.py | 24 ++ buildtrigger/test/test_githosthandler.py | 125 +++++++++ buildtrigger/test/test_githubhandler.py | 291 +-------------------- 5 files changed, 326 insertions(+), 287 deletions(-) create mode 100644 buildtrigger/test/__init__.py create mode 100644 buildtrigger/test/githubmock.py create mode 100644 buildtrigger/test/test_bitbuckethandler.py create mode 100644 buildtrigger/test/test_githosthandler.py diff --git a/buildtrigger/test/__init__.py b/buildtrigger/test/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/buildtrigger/test/githubmock.py b/buildtrigger/test/githubmock.py new file mode 100644 index 000000000..a5b22bf87 --- /dev/null +++ b/buildtrigger/test/githubmock.py @@ -0,0 +1,173 @@ +from datetime import datetime +from mock import Mock + +from github import GithubException + +from buildtrigger.githubhandler import GithubBuildTrigger +from util.morecollections import AttrDict + +def get_github_trigger(subdir=''): + trigger_obj = AttrDict(dict(auth_token='foobar', id='sometrigger')) + trigger = GithubBuildTrigger(trigger_obj, {'build_source': 'foo', 'subdir': subdir}) + trigger._get_client = get_mock_github + return trigger + +def get_mock_github(): + def get_commit_mock(commit_sha): + if commit_sha == 'aaaaaaa': + commit_mock = Mock() + commit_mock.sha = commit_sha + commit_mock.html_url = 'http://url/to/commit' + commit_mock.last_modified = 'now' + + commit_mock.commit = Mock() + commit_mock.commit.message = 'some cool message' + + commit_mock.committer = Mock() + commit_mock.committer.login = 'someuser' + commit_mock.committer.avatar_url = 'avatarurl' + commit_mock.committer.html_url = 'htmlurl' + + commit_mock.author = Mock() + commit_mock.author.login = 'someuser' + commit_mock.author.avatar_url = 'avatarurl' + commit_mock.author.html_url = 'htmlurl' + return commit_mock + + raise GithubException(None, None) + + def get_branch_mock(branch_name): + if branch_name == 'master': + branch_mock = Mock() + branch_mock.commit = Mock() + branch_mock.commit.sha = 'aaaaaaa' + return branch_mock + + raise GithubException(None, None) + + def get_repo_mock(namespace, name): + repo_mock = Mock() + repo_mock.owner = Mock() + repo_mock.owner.login = namespace + + repo_mock.full_name = '%s/%s' % (namespace, name) + repo_mock.name = name + repo_mock.description = 'some %s repo' % (name) + repo_mock.pushed_at = datetime.utcfromtimestamp(0) + repo_mock.html_url = 'http://some/url' + repo_mock.private = name == 'somerepo' + repo_mock.permissions = Mock() + repo_mock.permissions.admin = namespace == 'knownuser' + return repo_mock + + def get_user_repos_mock(): + return [get_repo_mock('knownuser', 'somerepo')] + + def get_org_repos_mock(type='all'): + return [get_repo_mock('someorg', 'somerepo'), get_repo_mock('someorg', 'anotherrepo')] + + def get_orgs_mock(): + return [get_org_mock('someorg')] + + def get_user_mock(username='knownuser'): + if username == 'knownuser': + user_mock = Mock() + user_mock.name = username + user_mock.plan = Mock() + user_mock.plan.private_repos = 1 + user_mock.login = username + user_mock.html_url = 'htmlurl' + user_mock.avatar_url = 'avatarurl' + user_mock.get_repos = Mock(side_effect=get_user_repos_mock) + user_mock.get_orgs = Mock(side_effect=get_orgs_mock) + return user_mock + + raise GithubException(None, None) + + def get_org_mock(namespace): + if namespace == 'someorg': + org_mock = Mock() + org_mock.get_repos = Mock(side_effect=get_org_repos_mock) + org_mock.login = namespace + org_mock.html_url = 'htmlurl' + org_mock.avatar_url = 'avatarurl' + org_mock.name = namespace + org_mock.plan = Mock() + org_mock.plan.private_repos = 2 + return org_mock + + raise GithubException(None, None) + + def get_tags_mock(): + sometag = Mock() + sometag.name = 'sometag' + sometag.commit = get_commit_mock('aaaaaaa') + + someothertag = Mock() + someothertag.name = 'someothertag' + someothertag.commit = get_commit_mock('aaaaaaa') + return [sometag, someothertag] + + def get_branches_mock(): + master = Mock() + master.name = 'master' + master.commit = get_commit_mock('aaaaaaa') + + otherbranch = Mock() + otherbranch.name = 'otherbranch' + otherbranch.commit = get_commit_mock('aaaaaaa') + return [master, otherbranch] + + def get_file_contents_mock(filepath): + if filepath == '/Dockerfile': + m = Mock() + m.content = 'hello world' + return m + + if filepath == 'somesubdir/Dockerfile': + m = Mock() + m.content = 'hi universe' + return m + + raise GithubException(None, None) + + def get_git_tree_mock(commit_sha, recursive=False): + first_file = Mock() + first_file.type = 'blob' + first_file.path = 'Dockerfile' + + second_file = Mock() + second_file.type = 'other' + second_file.path = '/some/Dockerfile' + + third_file = Mock() + third_file.type = 'blob' + third_file.path = 'somesubdir/Dockerfile' + + t = Mock() + + if commit_sha == 'aaaaaaa': + t.tree = [ + first_file, second_file, third_file, + ] + else: + t.tree = [] + + return t + + repo_mock = Mock() + repo_mock.default_branch = 'master' + repo_mock.ssh_url = 'ssh_url' + + repo_mock.get_branch = Mock(side_effect=get_branch_mock) + repo_mock.get_tags = Mock(side_effect=get_tags_mock) + repo_mock.get_branches = Mock(side_effect=get_branches_mock) + repo_mock.get_commit = Mock(side_effect=get_commit_mock) + repo_mock.get_file_contents = Mock(side_effect=get_file_contents_mock) + repo_mock.get_git_tree = Mock(side_effect=get_git_tree_mock) + + gh_mock = Mock() + gh_mock.get_repo = Mock(return_value=repo_mock) + gh_mock.get_user = Mock(side_effect=get_user_mock) + gh_mock.get_organization = Mock(side_effect=get_org_mock) + return gh_mock diff --git a/buildtrigger/test/test_bitbuckethandler.py b/buildtrigger/test/test_bitbuckethandler.py new file mode 100644 index 000000000..b95619bf6 --- /dev/null +++ b/buildtrigger/test/test_bitbuckethandler.py @@ -0,0 +1,24 @@ +import pytest + +from mock import Mock +from datetime import datetime + +from buildtrigger.bitbuckethandler import BitbucketBuildTrigger +from buildtrigger.triggerutil import (InvalidPayloadException, SkipRequestException, + TriggerStartException, ValidationRequestException) +from endpoints.building import PreparedBuild +from util.morecollections import AttrDict + +@pytest.fixture +def bitbucket_trigger(): + return _get_bitbucket_trigger() + +def get_mock_bitbucket(): + client_mock = Mock() + return client_mock + +def _get_bitbucket_trigger(subdir=''): + trigger_obj = AttrDict(dict(auth_token='foobar', id='sometrigger')) + trigger = BitbucketBuildTrigger(trigger_obj, {'build_source': 'foo/bar', 'subdir': subdir}) + trigger._get_client = get_mock_bitbucket + return trigger diff --git a/buildtrigger/test/test_githosthandler.py b/buildtrigger/test/test_githosthandler.py new file mode 100644 index 000000000..6ed25bf4f --- /dev/null +++ b/buildtrigger/test/test_githosthandler.py @@ -0,0 +1,125 @@ +import pytest + +from buildtrigger.triggerutil import TriggerStartException +from buildtrigger.test.githubmock import get_github_trigger +from endpoints.building import PreparedBuild + +def github_trigger(): + return get_github_trigger() + +@pytest.fixture(params=[github_trigger()]) +def githost_trigger(request): + return request.param + +@pytest.mark.parametrize('run_parameters, expected_error, expected_message', [ + # No branch or tag specified: use the commit of the default branch. + ({}, None, None), + + # Invalid branch. + ({'refs': {'kind': 'branch', 'name': 'invalid'}}, TriggerStartException, + 'Could not find branch in repository'), + + # Invalid tag. + ({'refs': {'kind': 'tag', 'name': 'invalid'}}, TriggerStartException, + 'Could not find tag in repository'), + + # Valid branch. + ({'refs': {'kind': 'branch', 'name': 'master'}}, None, None), + + # Valid tag. + ({'refs': {'kind': 'tag', 'name': 'sometag'}}, None, None), +]) +def test_manual_start(run_parameters, expected_error, expected_message, githost_trigger): + if expected_error is not None: + with pytest.raises(expected_error) as ipe: + githost_trigger.manual_start(run_parameters) + assert ipe.value.message == expected_message + else: + assert isinstance(githost_trigger.manual_start(run_parameters), PreparedBuild) + + +@pytest.mark.parametrize('username, expected_response', [ + ('unknownuser', None), + ('knownuser', {'html_url': 'htmlurl', 'avatar_url': 'avatarurl'}), +]) +def test_lookup_user(username, expected_response, githost_trigger): + assert githost_trigger.lookup_user(username) == expected_response + + +@pytest.mark.parametrize('name, expected', [ + ('refs', [ + {'kind': 'branch', 'name': 'master'}, + {'kind': 'branch', 'name': 'otherbranch'}, + {'kind': 'tag', 'name': 'sometag'}, + {'kind': 'tag', 'name': 'someothertag'}, + ]), + ('tag_name', ['sometag', 'someothertag']), + ('branch_name', ['master', 'otherbranch']), + ('invalid', None) +]) +def test_list_field_values(name, expected, githost_trigger): + assert githost_trigger.list_field_values(name) == expected + + +def test_list_build_subdirs(githost_trigger): + assert githost_trigger.list_build_subdirs() == ['', 'somesubdir'] + + +def test_list_build_source_namespaces(githost_trigger): + namespaces_expected = [ + { + 'personal': True, + 'score': 1, + 'avatar_url': 'avatarurl', + 'id': 'knownuser', + 'title': 'knownuser' + }, + { + 'score': 2, + 'title': 'someorg', + 'personal': False, + 'url': 'htmlurl', + 'avatar_url': 'avatarurl', + 'id': 'someorg' + } + ] + assert githost_trigger.list_build_source_namespaces() == namespaces_expected + + +@pytest.mark.parametrize('namespace, expected', [ + ('', []), + ('unknown', []), + + ('knownuser', [ + { + 'last_updated': 0, 'name': 'somerepo', 'url': 'http://some/url', 'private': True, + 'full_name': 'knownuser/somerepo', 'has_admin_permissions': True, + 'description': 'some somerepo repo' + }]), + + ('someorg', [ + { + 'last_updated': 0, 'name': 'somerepo', 'url': 'http://some/url', + 'private': True, 'full_name': 'someorg/somerepo', 'has_admin_permissions': False, + 'description': 'some somerepo repo' + }, + { + 'last_updated': 0, 'name': 'anotherrepo', 'url': 'http://some/url', + 'private': False, 'full_name': 'someorg/anotherrepo', 'has_admin_permissions': False, + 'description': 'some anotherrepo repo' + }]), +]) +def test_list_build_sources_for_namespace(namespace, expected, githost_trigger): + # TODO: schema validation on the resulting namespaces. + assert githost_trigger.list_build_sources_for_namespace(namespace) == expected + + +def test_activate(githost_trigger): + config, private_key = githost_trigger.activate('http://some/url') + assert 'deploy_key_id' in config + assert 'hook_id' in config + assert 'private_key' in private_key + + +def test_deactivate(githost_trigger): + githost_trigger.deactivate() diff --git a/buildtrigger/test/test_githubhandler.py b/buildtrigger/test/test_githubhandler.py index 3d6555f81..aa2fdd244 100644 --- a/buildtrigger/test/test_githubhandler.py +++ b/buildtrigger/test/test_githubhandler.py @@ -1,185 +1,15 @@ import json import pytest -from github import GithubException -from mock import Mock -from datetime import datetime - -from buildtrigger.githubhandler import GithubBuildTrigger -from buildtrigger.triggerutil import (InvalidPayloadException, SkipRequestException, - TriggerStartException, ValidationRequestException) +from buildtrigger.test.githubmock import get_github_trigger +from buildtrigger.triggerutil import SkipRequestException, ValidationRequestException from endpoints.building import PreparedBuild from util.morecollections import AttrDict -def get_mock_github(): - def get_commit_mock(commit_sha): - if commit_sha == 'aaaaaaa': - commit_mock = Mock() - commit_mock.sha = commit_sha - commit_mock.html_url = 'http://url/to/commit' - commit_mock.last_modified = 'now' - - commit_mock.commit = Mock() - commit_mock.commit.message = 'some cool message' - - commit_mock.committer = Mock() - commit_mock.committer.login = 'someuser' - commit_mock.committer.avatar_url = 'avatarurl' - commit_mock.committer.html_url = 'htmlurl' - - commit_mock.author = Mock() - commit_mock.author.login = 'someuser' - commit_mock.author.avatar_url = 'avatarurl' - commit_mock.author.html_url = 'htmlurl' - return commit_mock - - raise GithubException(None, None) - - def get_branch_mock(branch_name): - if branch_name == 'master': - branch_mock = Mock() - branch_mock.commit = Mock() - branch_mock.commit.sha = 'aaaaaaa' - return branch_mock - - raise GithubException(None, None) - - def get_repo_mock(namespace, name): - repo_mock = Mock() - repo_mock.owner = Mock() - repo_mock.owner.login = namespace - - repo_mock.full_name = '%s/%s' % (namespace, name) - repo_mock.name = name - repo_mock.description = 'some %s repo' % (name) - repo_mock.pushed_at = datetime.utcfromtimestamp(0) - repo_mock.html_url = 'http://some/url' - repo_mock.private = name == 'somerepo' - repo_mock.permissions = Mock() - repo_mock.permissions.admin = namespace == 'knownuser' - return repo_mock - - def get_user_repos_mock(): - return [get_repo_mock('knownuser', 'somerepo')] - - def get_org_repos_mock(type='all'): - return [get_repo_mock('someorg', 'somerepo'), get_repo_mock('someorg', 'anotherrepo')] - - def get_orgs_mock(): - return [get_org_mock('someorg')] - - def get_user_mock(username='knownuser'): - if username == 'knownuser': - user_mock = Mock() - user_mock.name = username - user_mock.plan = Mock() - user_mock.plan.private_repos = 1 - user_mock.login = username - user_mock.html_url = 'htmlurl' - user_mock.avatar_url = 'avatarurl' - user_mock.get_repos = Mock(side_effect=get_user_repos_mock) - user_mock.get_orgs = Mock(side_effect=get_orgs_mock) - return user_mock - - raise GithubException(None, None) - - def get_org_mock(namespace): - if namespace == 'someorg': - org_mock = Mock() - org_mock.get_repos = Mock(side_effect=get_org_repos_mock) - org_mock.login = namespace - org_mock.html_url = 'htmlurl' - org_mock.avatar_url = 'avatarurl' - org_mock.name = namespace - org_mock.plan = Mock() - org_mock.plan.private_repos = 2 - return org_mock - - raise GithubException(None, None) - - def get_tags_mock(): - sometag = Mock() - sometag.name = 'sometag' - sometag.commit = get_commit_mock('aaaaaaa') - - someothertag = Mock() - someothertag.name = 'someothertag' - someothertag.commit = get_commit_mock('aaaaaaa') - return [sometag, someothertag] - - def get_branches_mock(): - master = Mock() - master.name = 'master' - master.commit = get_commit_mock('aaaaaaa') - - otherbranch = Mock() - otherbranch.name = 'otherbranch' - otherbranch.commit = get_commit_mock('aaaaaaa') - return [master, otherbranch] - - def get_file_contents_mock(filepath): - if filepath == '/Dockerfile': - m = Mock() - m.content = 'hello world' - return m - - if filepath == 'somesubdir/Dockerfile': - m = Mock() - m.content = 'hi universe' - return m - - raise GithubException(None, None) - - def get_git_tree_mock(commit_sha, recursive=False): - first_file = Mock() - first_file.type = 'blob' - first_file.path = 'Dockerfile' - - second_file = Mock() - second_file.type = 'other' - second_file.path = '/some/Dockerfile' - - third_file = Mock() - third_file.type = 'blob' - third_file.path = 'somesubdir/Dockerfile' - - t = Mock() - - if commit_sha == 'aaaaaaa': - t.tree = [ - first_file, second_file, third_file, - ] - else: - t.tree = [] - - return t - - repo_mock = Mock() - repo_mock.default_branch = 'master' - repo_mock.ssh_url = 'ssh_url' - - repo_mock.get_branch = Mock(side_effect=get_branch_mock) - repo_mock.get_tags = Mock(side_effect=get_tags_mock) - repo_mock.get_branches = Mock(side_effect=get_branches_mock) - repo_mock.get_commit = Mock(side_effect=get_commit_mock) - repo_mock.get_file_contents = Mock(side_effect=get_file_contents_mock) - repo_mock.get_git_tree = Mock(side_effect=get_git_tree_mock) - - gh_mock = Mock() - gh_mock.get_repo = Mock(return_value=repo_mock) - gh_mock.get_user = Mock(side_effect=get_user_mock) - gh_mock.get_organization = Mock(side_effect=get_org_mock) - return gh_mock - @pytest.fixture def github_trigger(): - return _get_github_trigger() + return get_github_trigger() -def _get_github_trigger(subdir=''): - trigger_obj = AttrDict(dict(auth_token='foobar', id='sometrigger')) - trigger = GithubBuildTrigger(trigger_obj, {'build_source': 'foo', 'subdir': subdir}) - trigger._get_client = get_mock_github - return trigger @pytest.mark.parametrize('payload, expected_error, expected_message', [ ('{"zen": true}', SkipRequestException, ""), @@ -237,125 +67,12 @@ def test_handle_trigger_request(github_trigger, payload, expected_error, expecte assert isinstance(github_trigger.handle_trigger_request(request), PreparedBuild) -@pytest.mark.parametrize('run_parameters, expected_error, expected_message', [ - # No branch or tag specified: use the commit of the default branch. - ({}, None, None), - - # Invalid branch. - ({'refs': {'kind': 'branch', 'name': 'invalid'}}, TriggerStartException, - 'Could not find branch in repository'), - - # Invalid tag. - ({'refs': {'kind': 'tag', 'name': 'invalid'}}, TriggerStartException, - 'Could not find tag in repository'), - - # Valid branch. - ({'refs': {'kind': 'branch', 'name': 'master'}}, None, None), - - # Valid tag. - ({'refs': {'kind': 'tag', 'name': 'sometag'}}, None, None), -]) -def test_manual_start(run_parameters, expected_error, expected_message, github_trigger): - if expected_error is not None: - with pytest.raises(expected_error) as ipe: - github_trigger.manual_start(run_parameters) - assert ipe.value.message == expected_message - else: - assert isinstance(github_trigger.manual_start(run_parameters), PreparedBuild) - - -@pytest.mark.parametrize('username, expected_response', [ - ('unknownuser', None), - ('knownuser', {'html_url': 'htmlurl', 'avatar_url': 'avatarurl'}), -]) -def test_lookup_user(username, expected_response, github_trigger): - assert github_trigger.lookup_user(username) == expected_response - - -@pytest.mark.parametrize('name, expected', [ - ('refs', [ - {'kind': 'branch', 'name': 'master'}, - {'kind': 'branch', 'name': 'otherbranch'}, - {'kind': 'tag', 'name': 'sometag'}, - {'kind': 'tag', 'name': 'someothertag'}, - ]), - ('tag_name', ['sometag', 'someothertag']), - ('branch_name', ['master', 'otherbranch']), - ('invalid', None) -]) -def test_list_field_values(name, expected, github_trigger): - assert github_trigger.list_field_values(name) == expected - - @pytest.mark.parametrize('subdir, contents', [ ('', 'hello world'), ('somesubdir', 'hi universe'), ('unknownpath', None), ]) def test_load_dockerfile_contents(subdir, contents): - trigger = _get_github_trigger(subdir) + trigger = get_github_trigger(subdir) assert trigger.load_dockerfile_contents() == contents - -def test_list_build_subdirs(github_trigger): - assert github_trigger.list_build_subdirs() == ['', 'somesubdir'] - - -def test_list_build_source_namespaces(github_trigger): - namespaces_expected = [ - { - 'personal': True, - 'score': 1, - 'avatar_url': 'avatarurl', - 'id': 'knownuser', - 'title': 'knownuser' - }, - { - 'score': 2, - 'title': 'someorg', - 'personal': False, - 'url': 'htmlurl', - 'avatar_url': 'avatarurl', - 'id': 'someorg' - } - ] - assert github_trigger.list_build_source_namespaces() == namespaces_expected - - -@pytest.mark.parametrize('namespace, expected', [ - ('', []), - ('unknown', []), - - ('knownuser', [ - { - 'last_updated': 0, 'name': 'somerepo', 'url': 'http://some/url', 'private': True, - 'full_name': 'knownuser/somerepo', 'has_admin_permissions': True, - 'description': 'some somerepo repo' - }]), - - ('someorg', [ - { - 'last_updated': 0, 'name': 'somerepo', 'url': 'http://some/url', - 'private': True, 'full_name': 'someorg/somerepo', 'has_admin_permissions': False, - 'description': 'some somerepo repo' - }, - { - 'last_updated': 0, 'name': 'anotherrepo', 'url': 'http://some/url', - 'private': False, 'full_name': 'someorg/anotherrepo', 'has_admin_permissions': False, - 'description': 'some anotherrepo repo' - }]), -]) -def test_list_build_sources_for_namespace(namespace, expected, github_trigger): - # TODO: schema validation on the resulting namespaces. - assert github_trigger.list_build_sources_for_namespace(namespace) == expected - - -def test_activate(github_trigger): - config, private_key = github_trigger.activate('http://some/url') - assert 'deploy_key_id' in config - assert 'hook_id' in config - assert 'private_key' in private_key - - -def test_deactivate(github_trigger): - github_trigger.deactivate() From 497c90e7ea90e21870af67d8e28c7e49eaacf230 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 13 Feb 2017 15:01:09 -0500 Subject: [PATCH 04/10] Add unit testing of bitbucket trigger handler --- buildtrigger/bitbuckethandler.py | 9 +- buildtrigger/githubhandler.py | 1 + buildtrigger/test/bitbucketmock.py | 161 +++++++++++++++++++++ buildtrigger/test/githubmock.py | 6 +- buildtrigger/test/test_bitbuckethandler.py | 32 ++-- buildtrigger/test/test_githosthandler.py | 48 +++--- buildtrigger/test/test_githubhandler.py | 11 ++ 7 files changed, 217 insertions(+), 51 deletions(-) create mode 100644 buildtrigger/test/bitbucketmock.py diff --git a/buildtrigger/bitbuckethandler.py b/buildtrigger/bitbuckethandler.py index f96a38cee..33c890083 100644 --- a/buildtrigger/bitbuckethandler.py +++ b/buildtrigger/bitbuckethandler.py @@ -412,7 +412,8 @@ class BitbucketBuildTrigger(BuildTriggerHandler): 'id': owner, 'title': owner, 'avatar_url': repo['logo'], - 'score': 0, + 'url': 'https://bitbucket.org/%s' % (owner), + 'score': 1, } return list(namespaces.values()) @@ -464,7 +465,7 @@ class BitbucketBuildTrigger(BuildTriggerHandler): (result, data, err_msg) = repository.get_raw_path_contents(path, revision='master') if not result: - raise RepositoryReadException(err_msg) + return None return data @@ -541,7 +542,7 @@ class BitbucketBuildTrigger(BuildTriggerHandler): # Lookup the commit SHA for the branch. (result, data, _) = repository.get_branch(branch_name) if not result: - raise TriggerStartException('Could not find branch commit SHA') + raise TriggerStartException('Could not find branch in repository') return data['target']['hash'] @@ -549,7 +550,7 @@ class BitbucketBuildTrigger(BuildTriggerHandler): # Lookup the commit SHA for the tag. (result, data, _) = repository.get_tag(tag_name) if not result: - raise TriggerStartException('Could not find tag commit SHA') + raise TriggerStartException('Could not find tag in repository') return data['target']['hash'] diff --git a/buildtrigger/githubhandler.py b/buildtrigger/githubhandler.py index a51526507..3233f5f5d 100644 --- a/buildtrigger/githubhandler.py +++ b/buildtrigger/githubhandler.py @@ -286,6 +286,7 @@ class GithubBuildTrigger(BuildTriggerHandler): 'id': usr.login, 'title': usr.name or usr.login, 'avatar_url': usr.avatar_url, + 'url': usr.html_url, 'score': usr.plan.private_repos if usr.plan else 0, } diff --git a/buildtrigger/test/bitbucketmock.py b/buildtrigger/test/bitbucketmock.py new file mode 100644 index 000000000..c442f20dd --- /dev/null +++ b/buildtrigger/test/bitbucketmock.py @@ -0,0 +1,161 @@ +from datetime import datetime +from mock import Mock + +from buildtrigger.bitbuckethandler import BitbucketBuildTrigger +from util.morecollections import AttrDict + +def get_bitbucket_trigger(subdir=''): + trigger_obj = AttrDict(dict(auth_token='foobar', id='sometrigger')) + trigger = BitbucketBuildTrigger(trigger_obj, { + 'build_source': 'foo/bar', + 'subdir': subdir, + 'username': 'knownuser' + }) + + trigger._get_client = get_mock_bitbucket + return trigger + +def get_repo_path_contents(path, revision): + if revision != 'master': + return (False, None, None) + + data = { + 'files': [{'path': 'Dockerfile'}], + } + + return (True, data, None) + +def get_raw_path_contents(path, revision): + if path == '/Dockerfile': + return (True, 'hello world', None) + + if path == 'somesubdir/Dockerfile': + return (True, 'hi universe', None) + + return (False, None, None) + +def get_branches_and_tags(): + data = { + 'branches': [{'name': 'master'}, {'name': 'otherbranch'}], + 'tags': [{'name': 'sometag'}, {'name': 'someothertag'}], + } + return (True, data, None) + +def get_branches(): + return (True, {'master': {}, 'otherbranch': {}}, None) + +def get_tags(): + return (True, {'sometag': {}, 'someothertag': {}}, None) + +def get_branch(branch_name): + if branch_name != 'master': + return (False, None, None) + + data = { + 'target': { + 'hash': 'aaaaaaa', + }, + } + + return (True, data, None) + +def get_tag(tag_name): + if tag_name != 'sometag': + return (False, None, None) + + data = { + 'target': { + 'hash': 'aaaaaaa', + }, + } + + return (True, data, None) + +def get_changeset_mock(commit_sha): + if commit_sha != 'aaaaaaa': + return (False, None, 'Not found') + + data = { + 'node': 'aaaaaaa', + 'message': 'some message', + 'timestamp': 'now', + 'raw_author': 'foo@bar.com', + } + + return (True, data, None) + +def get_changesets(): + changesets_mock = Mock() + changesets_mock.get = Mock(side_effect=get_changeset_mock) + return changesets_mock + +def get_deploykeys(): + deploykeys_mock = Mock() + deploykeys_mock.create = Mock(return_value=(True, {'pk': 'someprivatekey'}, None)) + deploykeys_mock.delete = Mock(return_value=(True, {}, None)) + return deploykeys_mock + +def get_webhooks(): + webhooks_mock = Mock() + webhooks_mock.create = Mock(return_value=(True, {'uuid': 'someuuid'}, None)) + webhooks_mock.delete = Mock(return_value=(True, {}, None)) + return webhooks_mock + +def get_repo_mock(name): + if name != 'bar': + return None + + repo_mock = Mock() + repo_mock.get_main_branch = Mock(return_value=(True, {'name': 'master'}, None)) + repo_mock.get_path_contents = Mock(side_effect=get_repo_path_contents) + repo_mock.get_raw_path_contents = Mock(side_effect=get_raw_path_contents) + repo_mock.get_branches_and_tags = Mock(side_effect=get_branches_and_tags) + repo_mock.get_branches = Mock(side_effect=get_branches) + repo_mock.get_tags = Mock(side_effect=get_tags) + repo_mock.get_branch = Mock(side_effect=get_branch) + repo_mock.get_tag = Mock(side_effect=get_tag) + + repo_mock.changesets = Mock(side_effect=get_changesets) + repo_mock.deploykeys = Mock(side_effect=get_deploykeys) + repo_mock.webhooks = Mock(side_effect=get_webhooks) + return repo_mock + +def get_repositories_mock(): + repos_mock = Mock() + repos_mock.get = Mock(side_effect=get_repo_mock) + return repos_mock + +def get_namespace_mock(namespace): + namespace_mock = Mock() + namespace_mock.repositories = Mock(side_effect=get_repositories_mock) + return namespace_mock + +def get_repo(namespace, name): + return { + 'owner': namespace, + 'logo': 'avatarurl', + 'slug': name, + 'description': 'some %s repo' % (name), + 'utc_last_updated': str(datetime.utcfromtimestamp(0)), + 'read_only': namespace != 'knownuser', + 'is_private': name == 'somerepo', + } + +def get_visible_repos(): + repos = [ + get_repo('knownuser', 'somerepo'), + get_repo('someorg', 'somerepo'), + get_repo('someorg', 'anotherrepo'), + ] + return (True, repos, None) + +def get_authed_mock(token, secret): + authed_mock = Mock() + authed_mock.for_namespace = Mock(side_effect=get_namespace_mock) + authed_mock.get_visible_repositories = Mock(side_effect=get_visible_repos) + return authed_mock + +def get_mock_bitbucket(): + bitbucket_mock = Mock() + bitbucket_mock.get_authorized_client = Mock(side_effect=get_authed_mock) + return bitbucket_mock diff --git a/buildtrigger/test/githubmock.py b/buildtrigger/test/githubmock.py index a5b22bf87..77c8a7a1f 100644 --- a/buildtrigger/test/githubmock.py +++ b/buildtrigger/test/githubmock.py @@ -54,7 +54,7 @@ def get_mock_github(): repo_mock.name = name repo_mock.description = 'some %s repo' % (name) repo_mock.pushed_at = datetime.utcfromtimestamp(0) - repo_mock.html_url = 'http://some/url' + repo_mock.html_url = 'https://bitbucket.org/%s/%s' % (namespace, name) repo_mock.private = name == 'somerepo' repo_mock.permissions = Mock() repo_mock.permissions.admin = namespace == 'knownuser' @@ -76,7 +76,7 @@ def get_mock_github(): user_mock.plan = Mock() user_mock.plan.private_repos = 1 user_mock.login = username - user_mock.html_url = 'htmlurl' + user_mock.html_url = 'https://bitbucket.org/%s' % (username) user_mock.avatar_url = 'avatarurl' user_mock.get_repos = Mock(side_effect=get_user_repos_mock) user_mock.get_orgs = Mock(side_effect=get_orgs_mock) @@ -89,7 +89,7 @@ def get_mock_github(): org_mock = Mock() org_mock.get_repos = Mock(side_effect=get_org_repos_mock) org_mock.login = namespace - org_mock.html_url = 'htmlurl' + org_mock.html_url = 'https://bitbucket.org/%s' % (namespace) org_mock.avatar_url = 'avatarurl' org_mock.name = namespace org_mock.plan = Mock() diff --git a/buildtrigger/test/test_bitbuckethandler.py b/buildtrigger/test/test_bitbuckethandler.py index b95619bf6..991d90af1 100644 --- a/buildtrigger/test/test_bitbuckethandler.py +++ b/buildtrigger/test/test_bitbuckethandler.py @@ -1,24 +1,22 @@ import pytest -from mock import Mock -from datetime import datetime - -from buildtrigger.bitbuckethandler import BitbucketBuildTrigger -from buildtrigger.triggerutil import (InvalidPayloadException, SkipRequestException, - TriggerStartException, ValidationRequestException) -from endpoints.building import PreparedBuild -from util.morecollections import AttrDict +from buildtrigger.test.bitbucketmock import get_bitbucket_trigger @pytest.fixture def bitbucket_trigger(): - return _get_bitbucket_trigger() + return get_bitbucket_trigger() -def get_mock_bitbucket(): - client_mock = Mock() - return client_mock -def _get_bitbucket_trigger(subdir=''): - trigger_obj = AttrDict(dict(auth_token='foobar', id='sometrigger')) - trigger = BitbucketBuildTrigger(trigger_obj, {'build_source': 'foo/bar', 'subdir': subdir}) - trigger._get_client = get_mock_bitbucket - return trigger +def test_list_build_subdirs(bitbucket_trigger): + assert bitbucket_trigger.list_build_subdirs() == [''] + + +@pytest.mark.parametrize('subdir, contents', [ + ('', 'hello world'), + ('somesubdir', 'hi universe'), + ('unknownpath', None), +]) +def test_load_dockerfile_contents(subdir, contents): + trigger = get_bitbucket_trigger(subdir) + assert trigger.load_dockerfile_contents() == contents + diff --git a/buildtrigger/test/test_githosthandler.py b/buildtrigger/test/test_githosthandler.py index 6ed25bf4f..04b87fa3c 100644 --- a/buildtrigger/test/test_githosthandler.py +++ b/buildtrigger/test/test_githosthandler.py @@ -1,13 +1,11 @@ import pytest from buildtrigger.triggerutil import TriggerStartException +from buildtrigger.test.bitbucketmock import get_bitbucket_trigger from buildtrigger.test.githubmock import get_github_trigger from endpoints.building import PreparedBuild -def github_trigger(): - return get_github_trigger() - -@pytest.fixture(params=[github_trigger()]) +@pytest.fixture(params=[get_github_trigger(), get_bitbucket_trigger()]) def githost_trigger(request): return request.param @@ -38,14 +36,6 @@ def test_manual_start(run_parameters, expected_error, expected_message, githost_ assert isinstance(githost_trigger.manual_start(run_parameters), PreparedBuild) -@pytest.mark.parametrize('username, expected_response', [ - ('unknownuser', None), - ('knownuser', {'html_url': 'htmlurl', 'avatar_url': 'avatarurl'}), -]) -def test_lookup_user(username, expected_response, githost_trigger): - assert githost_trigger.lookup_user(username) == expected_response - - @pytest.mark.parametrize('name, expected', [ ('refs', [ {'kind': 'branch', 'name': 'master'}, @@ -53,16 +43,17 @@ def test_lookup_user(username, expected_response, githost_trigger): {'kind': 'tag', 'name': 'sometag'}, {'kind': 'tag', 'name': 'someothertag'}, ]), - ('tag_name', ['sometag', 'someothertag']), - ('branch_name', ['master', 'otherbranch']), + ('tag_name', set(['sometag', 'someothertag'])), + ('branch_name', set(['master', 'otherbranch'])), ('invalid', None) ]) def test_list_field_values(name, expected, githost_trigger): - assert githost_trigger.list_field_values(name) == expected - - -def test_list_build_subdirs(githost_trigger): - assert githost_trigger.list_build_subdirs() == ['', 'somesubdir'] + if expected is None: + assert githost_trigger.list_field_values(name) is None + elif isinstance(expected, set): + assert set(githost_trigger.list_field_values(name)) == set(expected) + else: + assert githost_trigger.list_field_values(name) == expected def test_list_build_source_namespaces(githost_trigger): @@ -72,13 +63,14 @@ def test_list_build_source_namespaces(githost_trigger): 'score': 1, 'avatar_url': 'avatarurl', 'id': 'knownuser', - 'title': 'knownuser' + 'title': 'knownuser', + 'url': 'https://bitbucket.org/knownuser', }, { 'score': 2, 'title': 'someorg', 'personal': False, - 'url': 'htmlurl', + 'url': 'https://bitbucket.org/someorg', 'avatar_url': 'avatarurl', 'id': 'someorg' } @@ -92,20 +84,23 @@ def test_list_build_source_namespaces(githost_trigger): ('knownuser', [ { - 'last_updated': 0, 'name': 'somerepo', 'url': 'http://some/url', 'private': True, + 'last_updated': 0, 'name': 'somerepo', + 'url': 'https://bitbucket.org/knownuser/somerepo', 'private': True, 'full_name': 'knownuser/somerepo', 'has_admin_permissions': True, 'description': 'some somerepo repo' }]), ('someorg', [ { - 'last_updated': 0, 'name': 'somerepo', 'url': 'http://some/url', - 'private': True, 'full_name': 'someorg/somerepo', 'has_admin_permissions': False, + 'last_updated': 0, 'name': 'somerepo', + 'url': 'https://bitbucket.org/someorg/somerepo', 'private': True, + 'full_name': 'someorg/somerepo', 'has_admin_permissions': False, 'description': 'some somerepo repo' }, { - 'last_updated': 0, 'name': 'anotherrepo', 'url': 'http://some/url', - 'private': False, 'full_name': 'someorg/anotherrepo', 'has_admin_permissions': False, + 'last_updated': 0, 'name': 'anotherrepo', + 'url': 'https://bitbucket.org/someorg/anotherrepo', 'private': False, + 'full_name': 'someorg/anotherrepo', 'has_admin_permissions': False, 'description': 'some anotherrepo repo' }]), ]) @@ -117,7 +112,6 @@ def test_list_build_sources_for_namespace(namespace, expected, githost_trigger): def test_activate(githost_trigger): config, private_key = githost_trigger.activate('http://some/url') assert 'deploy_key_id' in config - assert 'hook_id' in config assert 'private_key' in private_key diff --git a/buildtrigger/test/test_githubhandler.py b/buildtrigger/test/test_githubhandler.py index aa2fdd244..5f9a1d786 100644 --- a/buildtrigger/test/test_githubhandler.py +++ b/buildtrigger/test/test_githubhandler.py @@ -76,3 +76,14 @@ def test_load_dockerfile_contents(subdir, contents): trigger = get_github_trigger(subdir) assert trigger.load_dockerfile_contents() == contents + +@pytest.mark.parametrize('username, expected_response', [ + ('unknownuser', None), + ('knownuser', {'html_url': 'https://bitbucket.org/knownuser', 'avatar_url': 'avatarurl'}), +]) +def test_lookup_user(username, expected_response, github_trigger): + assert github_trigger.lookup_user(username) == expected_response + + +def test_list_build_subdirs(github_trigger): + assert github_trigger.list_build_subdirs() == ['', 'somesubdir'] From 84b298f36bc896b94472abbc674c74f1f2aa2d3f Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 13 Feb 2017 16:17:52 -0500 Subject: [PATCH 05/10] Add missing bitbucket test --- buildtrigger/bitbuckethandler.py | 26 ++++---- buildtrigger/test/test_bitbuckethandler.py | 69 ++++++++++++++++++++++ 2 files changed, 82 insertions(+), 13 deletions(-) diff --git a/buildtrigger/bitbuckethandler.py b/buildtrigger/bitbuckethandler.py index 33c890083..2f26626a9 100644 --- a/buildtrigger/bitbuckethandler.py +++ b/buildtrigger/bitbuckethandler.py @@ -35,7 +35,7 @@ BITBUCKET_WEBHOOK_PAYLOAD_SCHEMA = { }, }, 'required': ['full_name'], - }, + }, # /Repository 'push': { 'type': 'object', 'properties': { @@ -91,10 +91,10 @@ BITBUCKET_WEBHOOK_PAYLOAD_SCHEMA = { }, }, 'required': ['html', 'avatar'], - }, + }, # /User }, 'required': ['username'], - }, + }, # /Author }, }, 'links': { @@ -111,19 +111,19 @@ BITBUCKET_WEBHOOK_PAYLOAD_SCHEMA = { }, }, 'required': ['html'], - }, + }, # /Links }, 'required': ['hash', 'message', 'date'], - }, + }, # /Target }, - 'required': ['target'], - }, + 'required': ['name', 'target'], + }, # /New }, - }, - }, + }, # /Changes item + }, # /Changes }, 'required': ['changes'], - }, + }, # / Push }, 'actor': { 'type': 'object', @@ -157,9 +157,9 @@ BITBUCKET_WEBHOOK_PAYLOAD_SCHEMA = { }, }, 'required': ['username'], - }, + }, # /Actor 'required': ['push', 'repository'], -} +} # /Root BITBUCKET_COMMIT_INFO_SCHEMA = { 'type': 'object', @@ -242,7 +242,7 @@ def get_transformed_webhook_payload(bb_payload, default_branch=None): config['default_branch'] = default_branch config['git_url'] = 'git@bitbucket.org:%s.git' % repository_name - config['commit_info.url'] = target['links.html.href'] + config['commit_info.url'] = target['links.html.href'] or '' config['commit_info.message'] = target['message'] config['commit_info.date'] = target['date'] diff --git a/buildtrigger/test/test_bitbuckethandler.py b/buildtrigger/test/test_bitbuckethandler.py index 991d90af1..12c653db5 100644 --- a/buildtrigger/test/test_bitbuckethandler.py +++ b/buildtrigger/test/test_bitbuckethandler.py @@ -1,6 +1,11 @@ +import json import pytest from buildtrigger.test.bitbucketmock import get_bitbucket_trigger +from buildtrigger.triggerutil import (SkipRequestException, ValidationRequestException, + InvalidPayloadException) +from endpoints.building import PreparedBuild +from util.morecollections import AttrDict @pytest.fixture def bitbucket_trigger(): @@ -20,3 +25,67 @@ def test_load_dockerfile_contents(subdir, contents): trigger = get_bitbucket_trigger(subdir) assert trigger.load_dockerfile_contents() == contents + +@pytest.mark.parametrize('payload, expected_error, expected_message', [ + ('{}', InvalidPayloadException, "'push' is a required property"), + + # Valid payload: + ('''{ + "push": { + "changes": [{ + "new": { + "name": "somechange", + "target": { + "hash": "aaaaaaa", + "message": "foo", + "date": "now", + "links": { + "html": { + "href": "somelink" + } + } + } + } + }] + }, + "repository": { + "full_name": "foo/bar" + } + }''', None, None), + + # Skip message: + ('''{ + "push": { + "changes": [{ + "new": { + "name": "somechange", + "target": { + "hash": "aaaaaaa", + "message": "[skip build] foo", + "date": "now", + "links": { + "html": { + "href": "somelink" + } + } + } + } + }] + }, + "repository": { + "full_name": "foo/bar" + } + }''', SkipRequestException, ''), +]) +def test_handle_trigger_request(bitbucket_trigger, payload, expected_error, expected_message): + def get_payload(): + return json.loads(payload) + + request = AttrDict(dict(get_json=get_payload)) + + if expected_error is not None: + with pytest.raises(expected_error) as ipe: + bitbucket_trigger.handle_trigger_request(request) + assert ipe.value.message == expected_message + else: + assert isinstance(bitbucket_trigger.handle_trigger_request(request), PreparedBuild) From 57528aa2bc6772d5a6b89b98c3c54232dc648187 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 13 Feb 2017 17:42:37 -0500 Subject: [PATCH 06/10] Add unit testing of gitlab trigger handler --- buildtrigger/gitlabhandler.py | 14 +- buildtrigger/test/gitlabmock.py | 186 +++++++++++++++++++++++ buildtrigger/test/test_githosthandler.py | 6 +- buildtrigger/test/test_gitlabhandler.py | 88 +++++++++++ 4 files changed, 287 insertions(+), 7 deletions(-) create mode 100644 buildtrigger/test/gitlabmock.py create mode 100644 buildtrigger/test/test_gitlabhandler.py diff --git a/buildtrigger/gitlabhandler.py b/buildtrigger/gitlabhandler.py index 19d701ca0..90be9f698 100644 --- a/buildtrigger/gitlabhandler.py +++ b/buildtrigger/gitlabhandler.py @@ -48,6 +48,9 @@ GITLAB_WEBHOOK_PAYLOAD_SCHEMA = { 'items': { 'type': 'object', 'properties': { + 'id': { + 'type': 'string', + }, 'url': { 'type': 'string', }, @@ -67,7 +70,7 @@ GITLAB_WEBHOOK_PAYLOAD_SCHEMA = { 'required': ['email'], }, }, - 'required': ['url', 'message', 'timestamp'], + 'required': ['id', 'url', 'message', 'timestamp'], }, }, }, @@ -282,7 +285,8 @@ class GitLabBuildTrigger(BuildTriggerHandler): 'id': namespace['path'], 'title': namespace['name'], 'avatar_url': repo['owner']['avatar_url'], - 'score': 0, + 'score': 1, + 'url': gl_client.host + '/' + namespace['path'], } return list(namespaces.values()) @@ -486,18 +490,18 @@ class GitLabBuildTrigger(BuildTriggerHandler): def get_tag_sha(tag_name): tags = gl_client.getrepositorytags(repo['id']) if tags is False: - raise TriggerStartException('Could not find tags') + raise TriggerStartException('Could not find tag in repository') for tag in tags: if tag['name'] == tag_name: return tag['commit']['id'] - raise TriggerStartException('Could not find commit') + raise TriggerStartException('Could not find tag in repository') def get_branch_sha(branch_name): branch = gl_client.getbranch(repo['id'], branch_name) if branch is False: - raise TriggerStartException('Could not find branch') + raise TriggerStartException('Could not find branch in repository') return branch['commit']['id'] diff --git a/buildtrigger/test/gitlabmock.py b/buildtrigger/test/gitlabmock.py new file mode 100644 index 000000000..8a2212be9 --- /dev/null +++ b/buildtrigger/test/gitlabmock.py @@ -0,0 +1,186 @@ +from datetime import datetime +from mock import Mock + +from buildtrigger.gitlabhandler import GitLabBuildTrigger +from util.morecollections import AttrDict + +def get_gitlab_trigger(subdir=''): + trigger_obj = AttrDict(dict(auth_token='foobar', id='sometrigger')) + trigger = GitLabBuildTrigger(trigger_obj, { + 'build_source': 'foo/bar', + 'subdir': subdir, + 'username': 'knownuser' + }) + + trigger._get_authorized_client = get_mock_gitlab + return trigger + +def adddeploykey_mock(project_id, name, public_key): + return {'id': 'foo'} + +def addprojecthook_mock(project_id, webhook_url, push=False): + return {'id': 'foo'} + +def get_currentuser_mock(): + return { + 'username': 'knownuser' + } + +def project(namespace, name): + return { + 'id': '%s/%s' % (namespace, name), + 'default_branch': 'master', + 'namespace': { + 'id': namespace, + 'path': namespace, + 'name': namespace, + }, + 'path': name, + 'path_with_namespace': '%s/%s' % (namespace, name), + 'description': 'some %s repo' % name, + 'last_activity_at': str(datetime.utcfromtimestamp(0)), + 'web_url': 'https://bitbucket.org/%s/%s' % (namespace, name), + 'ssh_url_to_repo': 'git://%s/%s' % (namespace, name), + 'public': name != 'somerepo', + 'permissions': { + 'project_access': { + 'access_level': 50 if namespace == 'knownuser' else 0, + } + }, + 'owner': { + 'avatar_url': 'avatarurl', + } + } + +def getprojects_mock(page=1, per_page=100): + return [ + project('knownuser', 'somerepo'), + project('someorg', 'somerepo'), + project('someorg', 'anotherrepo'), + ] + +def getproject_mock(project_name): + if project_name == 'knownuser/somerepo': + return project('knownuser', 'somerepo') + + if project_name == 'foo/bar': + return project('foo', 'bar') + + return False + + +def getbranches_mock(project_id): + return [ + { + 'name': 'master', + 'commit': { + 'id': 'aaaaaaa', + } + }, + { + 'name': 'otherbranch', + 'commit': { + 'id': 'aaaaaaa', + } + }, + ] + +def getrepositorytags_mock(project_id): + return [ + { + 'name': 'sometag', + 'commit': { + 'id': 'aaaaaaa', + } + }, + { + 'name': 'someothertag', + 'commit': { + 'id': 'aaaaaaa', + } + }, + ] + +def getrepositorytree_mock(project_id, ref_name='master'): + return [ + {'name': 'README'}, + {'name': 'Dockerfile'}, + ] + +def getrepositorycommit_mock(project_id, commit_sha): + if commit_sha != 'aaaaaaa': + return False + + return { + 'id': 'aaaaaaa', + 'message': 'some message', + 'committed_date': 'now', + } + +def getusers_mock(search=None): + if search == 'knownuser': + return [ + { + 'username': 'knownuser', + 'avatar_url': 'avatarurl', + } + ] + + return False + +def getbranch_mock(repo_id, branch): + if branch != 'master' and branch != 'otherbranch': + return False + + return { + 'name': branch, + 'commit': { + 'id': 'aaaaaaa', + } + } + +def gettag_mock(repo_id, tag): + if tag != 'sometag' and tag != 'someothertag': + return False + + return { + 'name': tag, + 'commit': { + 'id': 'aaaaaaa', + } + } + +def getrawfile_mock(repo_id, branch_name, path): + if path == '/Dockerfile': + return 'hello world' + + if path == 'somesubdir/Dockerfile': + return 'hi universe' + + return False + +def get_mock_gitlab(): + mock_gitlab = Mock() + mock_gitlab.host = 'https://bitbucket.org' + + mock_gitlab.currentuser = Mock(side_effect=get_currentuser_mock) + mock_gitlab.getusers = Mock(side_effect=getusers_mock) + + mock_gitlab.getprojects = Mock(side_effect=getprojects_mock) + mock_gitlab.getproject = Mock(side_effect=getproject_mock) + mock_gitlab.getbranches = Mock(side_effect=getbranches_mock) + + mock_gitlab.getbranch = Mock(side_effect=getbranch_mock) + mock_gitlab.gettag = Mock(side_effect=gettag_mock) + + mock_gitlab.getrepositorytags = Mock(side_effect=getrepositorytags_mock) + mock_gitlab.getrepositorytree = Mock(side_effect=getrepositorytree_mock) + mock_gitlab.getrepositorycommit = Mock(side_effect=getrepositorycommit_mock) + + mock_gitlab.getrawfile = Mock(side_effect=getrawfile_mock) + + mock_gitlab.adddeploykey = Mock(side_effect=adddeploykey_mock) + mock_gitlab.addprojecthook = Mock(side_effect=addprojecthook_mock) + mock_gitlab.deletedeploykey = Mock(return_value=True) + mock_gitlab.deleteprojecthook = Mock(return_value=True) + return mock_gitlab diff --git a/buildtrigger/test/test_githosthandler.py b/buildtrigger/test/test_githosthandler.py index 04b87fa3c..2cf4f9175 100644 --- a/buildtrigger/test/test_githosthandler.py +++ b/buildtrigger/test/test_githosthandler.py @@ -3,9 +3,12 @@ import pytest from buildtrigger.triggerutil import TriggerStartException from buildtrigger.test.bitbucketmock import get_bitbucket_trigger from buildtrigger.test.githubmock import get_github_trigger +from buildtrigger.test.gitlabmock import get_gitlab_trigger from endpoints.building import PreparedBuild -@pytest.fixture(params=[get_github_trigger(), get_bitbucket_trigger()]) +# Note: This test suite executes a common set of tests against all the trigger types specified +# in this fixture. Each trigger's mock is expected to return the same data for all of these calls. +@pytest.fixture(params=[get_github_trigger(), get_bitbucket_trigger(), get_gitlab_trigger()]) def githost_trigger(request): return request.param @@ -111,7 +114,6 @@ def test_list_build_sources_for_namespace(namespace, expected, githost_trigger): def test_activate(githost_trigger): config, private_key = githost_trigger.activate('http://some/url') - assert 'deploy_key_id' in config assert 'private_key' in private_key diff --git a/buildtrigger/test/test_gitlabhandler.py b/buildtrigger/test/test_gitlabhandler.py new file mode 100644 index 000000000..25170cbca --- /dev/null +++ b/buildtrigger/test/test_gitlabhandler.py @@ -0,0 +1,88 @@ +import json +import pytest + +from buildtrigger.test.gitlabmock import get_gitlab_trigger +from buildtrigger.triggerutil import (SkipRequestException, ValidationRequestException, + InvalidPayloadException) +from endpoints.building import PreparedBuild +from util.morecollections import AttrDict + +@pytest.fixture +def gitlab_trigger(): + return get_gitlab_trigger() + + +def test_list_build_subdirs(gitlab_trigger): + assert gitlab_trigger.list_build_subdirs() == [''] + + +@pytest.mark.parametrize('subdir, contents', [ + ('', 'hello world'), + ('somesubdir', 'hi universe'), + ('unknownpath', None), +]) +def test_load_dockerfile_contents(subdir, contents): + trigger = get_gitlab_trigger(subdir) + assert trigger.load_dockerfile_contents() == contents + + +@pytest.mark.parametrize('email, expected_response', [ + ('unknown@email.com', None), + ('knownuser', {'username': 'knownuser', 'html_url': 'https://bitbucket.org/knownuser', + 'avatar_url': 'avatarurl'}), +]) +def test_lookup_user(email, expected_response, gitlab_trigger): + assert gitlab_trigger.lookup_user(email) == expected_response + + +@pytest.mark.parametrize('payload, expected_error, expected_message', [ + ('{}', SkipRequestException, ''), + + # Valid payload: + ('''{ + "ref": "refs/heads/master", + "checkout_sha": "aaaaaaa", + "repository": { + "git_ssh_url": "foobar" + }, + "commits": [ + { + "id": "aaaaaaa", + "url": "someurl", + "message": "hello there!", + "timestamp": "now" + } + ] + }''', None, None), + + # Skip message: + ('''{ + "ref": "refs/heads/master", + "checkout_sha": "aaaaaaa", + "repository": { + "git_ssh_url": "foobar" + }, + "commits": [ + { + "id": "aaaaaaa", + "url": "someurl", + "message": "[skip build] hello there!", + "timestamp": "now" + } + ] + }''', SkipRequestException, ''), +]) +def test_handle_trigger_request(gitlab_trigger, payload, expected_error, expected_message): + def get_payload(): + return json.loads(payload) + + request = AttrDict(dict(get_json=get_payload)) + + if expected_error is not None: + with pytest.raises(expected_error) as ipe: + gitlab_trigger.handle_trigger_request(request) + assert ipe.value.message == expected_message + else: + assert isinstance(gitlab_trigger.handle_trigger_request(request), PreparedBuild) + + From e025d8c2b28492fd9b25748c20f5f8b31c05f01a Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 13 Feb 2017 18:04:41 -0500 Subject: [PATCH 07/10] Add schema validation of namespaces and sources methods --- buildtrigger/basehandler.py | 84 ++++++++++++++++++++++++ buildtrigger/bitbuckethandler.py | 5 +- buildtrigger/githubhandler.py | 8 ++- buildtrigger/gitlabhandler.py | 5 +- buildtrigger/test/test_githosthandler.py | 3 +- 5 files changed, 96 insertions(+), 9 deletions(-) diff --git a/buildtrigger/basehandler.py b/buildtrigger/basehandler.py index b9f035dc0..f8ed97563 100644 --- a/buildtrigger/basehandler.py +++ b/buildtrigger/basehandler.py @@ -6,6 +6,79 @@ from endpoints.building import PreparedBuild from data import model from buildtrigger.triggerutil import get_trigger_config, InvalidServiceException +NAMESPACES_SCHEMA = { + 'type': 'array', + 'items': { + 'type': 'object', + 'properties': { + 'personal': { + 'type': 'boolean', + 'description': 'True if the namespace is the user\'s personal namespace', + }, + 'score': { + 'type': 'number', + 'description': 'Score of the relevance of the namespace', + }, + 'avatar_url': { + 'type': 'string', + 'description': 'URL of the avatar for this namespace', + }, + 'url': { + 'type': 'string', + 'description': 'URL of the website to view the namespace', + }, + 'id': { + 'type': 'string', + 'description': 'Trigger-internal ID of the namespace', + }, + 'title': { + 'type': 'string', + 'description': 'Human-readable title of the namespace', + }, + }, + 'required': ['personal', 'score', 'avatar_url', 'url', 'id', 'title'], + }, +} + +BUILD_SOURCES_SCHEMA = { + 'type': 'array', + 'items': { + 'type': 'object', + 'properties': { + 'name': { + 'type': 'string', + 'description': 'The name of the repository, without its namespace', + }, + 'full_name': { + 'type': 'string', + 'description': 'The name of the repository, with its namespace', + }, + 'description': { + 'type': 'string', + 'description': 'The description of the repository. May be an empty string', + }, + 'last_updated': { + 'type': 'number', + 'description': 'The date/time when the repository was last updated, since epoch in UTC', + }, + 'url': { + 'type': 'string', + 'description': 'The URL at which to view the repository in the browser', + }, + 'has_admin_permissions': { + 'type': 'boolean', + 'description': 'True if the current user has admin permissions on the repository', + }, + 'private': { + 'type': 'boolean', + 'description': 'True if the repository is private', + }, + }, + 'required': ['name', 'full_name', 'description', 'last_updated', 'url', + 'has_admin_permissions', 'private'], + }, +} + METADATA_SCHEMA = { 'type': 'object', 'properties': { @@ -242,3 +315,14 @@ class BuildTriggerHandler(object): prepared.tags = [commit_sha[:7]] return prepared + + @classmethod + def build_sources_response(cls, sources): + validate(sources, BUILD_SOURCES_SCHEMA) + return sources + + @classmethod + def build_namespaces_response(cls, namespaces_dict): + namespaces = list(namespaces_dict.values()) + validate(namespaces, NAMESPACES_SCHEMA) + return namespaces diff --git a/buildtrigger/bitbuckethandler.py b/buildtrigger/bitbuckethandler.py index 2f26626a9..02e6b228f 100644 --- a/buildtrigger/bitbuckethandler.py +++ b/buildtrigger/bitbuckethandler.py @@ -416,7 +416,7 @@ class BitbucketBuildTrigger(BuildTriggerHandler): 'score': 1, } - return list(namespaces.values()) + return BuildTriggerHandler.build_namespaces_response(namespaces) def list_build_sources_for_namespace(self, namespace): def repo_view(repo): @@ -437,7 +437,8 @@ class BitbucketBuildTrigger(BuildTriggerHandler): if not result: raise RepositoryReadException('Could not read repository list: ' + err_msg) - return [repo_view(repo) for repo in data if repo['owner'] == namespace] + repos = [repo_view(repo) for repo in data if repo['owner'] == namespace] + return BuildTriggerHandler.build_sources_response(repos) def list_build_subdirs(self): config = self.config diff --git a/buildtrigger/githubhandler.py b/buildtrigger/githubhandler.py index 3233f5f5d..b7eca92cc 100644 --- a/buildtrigger/githubhandler.py +++ b/buildtrigger/githubhandler.py @@ -300,7 +300,7 @@ class GithubBuildTrigger(BuildTriggerHandler): 'score': org.plan.private_repos if org.plan else 0, } - return list(namespaces.values()) + return BuildTriggerHandler.build_namespaces_response(namespaces) @_catch_ssl_errors def list_build_sources_for_namespace(self, namespace): @@ -318,7 +318,8 @@ class GithubBuildTrigger(BuildTriggerHandler): gh_client = self._get_client() usr = gh_client.get_user() if namespace == usr.login: - return [repo_view(repo) for repo in usr.get_repos() if repo.owner.login == namespace] + repos = [repo_view(repo) for repo in usr.get_repos() if repo.owner.login == namespace] + return BuildTriggerHandler.build_sources_response(repos) try: org = gh_client.get_organization(namespace) @@ -327,7 +328,8 @@ class GithubBuildTrigger(BuildTriggerHandler): except GithubException: return [] - return [repo_view(repo) for repo in org.get_repos(type='member')] + repos = [repo_view(repo) for repo in org.get_repos(type='member')] + return BuildTriggerHandler.build_sources_response(repos) @_catch_ssl_errors diff --git a/buildtrigger/gitlabhandler.py b/buildtrigger/gitlabhandler.py index 90be9f698..5d221874b 100644 --- a/buildtrigger/gitlabhandler.py +++ b/buildtrigger/gitlabhandler.py @@ -289,7 +289,7 @@ class GitLabBuildTrigger(BuildTriggerHandler): 'url': gl_client.host + '/' + namespace['path'], } - return list(namespaces.values()) + return BuildTriggerHandler.build_namespaces_response(namespaces) @_catch_timeouts def list_build_sources_for_namespace(self, namespace): @@ -313,7 +313,8 @@ class GitLabBuildTrigger(BuildTriggerHandler): gl_client = self._get_authorized_client() repositories = _paginated_iterator(gl_client.getprojects, RepositoryReadException) - return [repo_view(repo) for repo in repositories if repo['namespace']['path'] == namespace] + repos = [repo_view(repo) for repo in repositories if repo['namespace']['path'] == namespace] + return BuildTriggerHandler.build_sources_response(repos) @_catch_timeouts def list_build_subdirs(self): diff --git a/buildtrigger/test/test_githosthandler.py b/buildtrigger/test/test_githosthandler.py index 2cf4f9175..06e73578d 100644 --- a/buildtrigger/test/test_githosthandler.py +++ b/buildtrigger/test/test_githosthandler.py @@ -108,12 +108,11 @@ def test_list_build_source_namespaces(githost_trigger): }]), ]) def test_list_build_sources_for_namespace(namespace, expected, githost_trigger): - # TODO: schema validation on the resulting namespaces. assert githost_trigger.list_build_sources_for_namespace(namespace) == expected def test_activate(githost_trigger): - config, private_key = githost_trigger.activate('http://some/url') + _, private_key = githost_trigger.activate('http://some/url') assert 'private_key' in private_key From c9bddd9c9a3285b6f4e78b8595f63c1b86bc3b5a Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 13 Feb 2017 18:14:45 -0500 Subject: [PATCH 08/10] Remove unnecessary check --- buildtrigger/test/bitbucketmock.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/buildtrigger/test/bitbucketmock.py b/buildtrigger/test/bitbucketmock.py index c442f20dd..93d66d143 100644 --- a/buildtrigger/test/bitbucketmock.py +++ b/buildtrigger/test/bitbucketmock.py @@ -16,9 +16,6 @@ def get_bitbucket_trigger(subdir=''): return trigger def get_repo_path_contents(path, revision): - if revision != 'master': - return (False, None, None) - data = { 'files': [{'path': 'Dockerfile'}], } From b403906bc8a074a91d8075d386dab15c10618723 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 13 Feb 2017 18:32:32 -0500 Subject: [PATCH 09/10] Fix flakiness in new tests due to change in hash seed --- buildtrigger/test/test_customhandler.py | 2 +- buildtrigger/test/test_githosthandler.py | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/buildtrigger/test/test_customhandler.py b/buildtrigger/test/test_customhandler.py index 6d05cb2b9..03fcad9be 100644 --- a/buildtrigger/test/test_customhandler.py +++ b/buildtrigger/test/test_customhandler.py @@ -10,7 +10,7 @@ from util.morecollections import AttrDict ('', InvalidPayloadException, 'Missing expected payload'), ('{}', InvalidPayloadException, "'commit' is a required property"), - ('{"commit": "foo", "ref": "bar", "default_branch": "baz"}', + ('{"commit": "foo", "ref": "refs/heads/something", "default_branch": "baz"}', InvalidPayloadException, "u'foo' does not match '^([A-Fa-f0-9]{7,})$'"), ('{"commit": "11d6fbc", "ref": "refs/heads/something", "default_branch": "baz"}', None, None), diff --git a/buildtrigger/test/test_githosthandler.py b/buildtrigger/test/test_githosthandler.py index 06e73578d..53005d4c0 100644 --- a/buildtrigger/test/test_githosthandler.py +++ b/buildtrigger/test/test_githosthandler.py @@ -78,7 +78,12 @@ def test_list_build_source_namespaces(githost_trigger): 'id': 'someorg' } ] - assert githost_trigger.list_build_source_namespaces() == namespaces_expected + + found = githost_trigger.list_build_source_namespaces() + found.sort() + + namespaces_expected.sort() + assert found == namespaces_expected @pytest.mark.parametrize('namespace, expected', [ From c3edc3855a26373d484cbb738b51090c8caf5ca3 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Tue, 28 Feb 2017 17:13:00 -0500 Subject: [PATCH 10/10] Fix build trigger tests --- buildtrigger/githubhandler.py | 2 +- buildtrigger/test/test_gitlabhandler.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/buildtrigger/githubhandler.py b/buildtrigger/githubhandler.py index b7eca92cc..7c47df4ff 100644 --- a/buildtrigger/githubhandler.py +++ b/buildtrigger/githubhandler.py @@ -11,7 +11,7 @@ from github import (Github, UnknownObjectException, GithubException, from jsonschema import validate -from app import github_trigger +from app import app, github_trigger from buildtrigger.triggerutil import (RepositoryReadException, TriggerActivationException, TriggerDeactivationException, TriggerStartException, EmptyRepositoryException, ValidationRequestException, diff --git a/buildtrigger/test/test_gitlabhandler.py b/buildtrigger/test/test_gitlabhandler.py index 25170cbca..c88c591d6 100644 --- a/buildtrigger/test/test_gitlabhandler.py +++ b/buildtrigger/test/test_gitlabhandler.py @@ -40,6 +40,7 @@ def test_lookup_user(email, expected_response, gitlab_trigger): # Valid payload: ('''{ + "object_kind": "push", "ref": "refs/heads/master", "checkout_sha": "aaaaaaa", "repository": { @@ -57,6 +58,7 @@ def test_lookup_user(email, expected_response, gitlab_trigger): # Skip message: ('''{ + "object_kind": "push", "ref": "refs/heads/master", "checkout_sha": "aaaaaaa", "repository": {