From 59c574091518cebb3bd2eb69de055eafbcbbe179 Mon Sep 17 00:00:00 2001 From: Alex Couper Date: Tue, 7 May 2013 15:43:03 +0100 Subject: [PATCH 1/9] Have provider specific ids for issues. --- deploystream/apps/feature/lib.py | 12 +++++++++--- deploystream/apps/feature/views.py | 7 ++++--- tests/test_feature/test_view.py | 4 ++-- tests/test_oauth/test_get_token.py | 2 +- 4 files changed, 16 insertions(+), 9 deletions(-) diff --git a/deploystream/apps/feature/lib.py b/deploystream/apps/feature/lib.py index b62b206..d73f10d 100644 --- a/deploystream/apps/feature/lib.py +++ b/deploystream/apps/feature/lib.py @@ -22,13 +22,19 @@ def get_all_features(providers): return all_features -def get_feature_info(feature_id, providers): +def get_feature_info(feature_provider, feature_id, providers): """ Get the information associated with the given feature from the providers given. - ``planning``, ``source_code`` and ``build_info`` are relevant providers to - be called. + :param feature_provider: + The name of the planning provider who knows of this feature. + + :param feature_id: + The planning-provider specific id for the feature. + + :param providers: + A dictionary of all providers. """ # TODO: since features may come from various origins, we need # at this stage to either use a feature id that is a string such as diff --git a/deploystream/apps/feature/views.py b/deploystream/apps/feature/views.py index ad6b34f..390e85c 100644 --- a/deploystream/apps/feature/views.py +++ b/deploystream/apps/feature/views.py @@ -29,9 +29,10 @@ def list_features(providers): return features -@app.route('/features/', methods=['GET']) +@app.route('/features/', methods=['GET']) @needs_providers @as_json -def view_feature(feature_id, providers): - feature = get_feature_info(feature_id, providers) +def view_feature(provider_feature_id, providers): + provider_id, feature_id = provider_feature_id.split('...') + feature = get_feature_info(provider_id, feature_id, providers) return feature diff --git a/tests/test_feature/test_view.py b/tests/test_feature/test_view.py index 73233ed..ebcd4b3 100644 --- a/tests/test_feature/test_view.py +++ b/tests/test_feature/test_view.py @@ -64,7 +64,7 @@ def setUp(self): 'testsource': SourceCodeProvider, 'testbuild': BuildInfoProvider}) def test_feature_view_shows_details(self): - response = self.client.get('/features/FT101') + response = self.client.get('/features/plan...FT101') assert "Amazing feature that will blow your mind" in response.data @patch("deploystream.providers.ALL_PROVIDER_CLASSES", @@ -75,5 +75,5 @@ def test_only_uses_providers_user_specifies(self): conf = deploystream.app.config del conf['USER_SPECIFIC_INFO']['provider_config'][0] - response = self.client.get('/features/FT101') + response = self.client.get('/features/plan...FT101') assert "Amazing feature that will blow your mind" not in response.data diff --git a/tests/test_oauth/test_get_token.py b/tests/test_oauth/test_get_token.py index f1cb16a..1c618a7 100644 --- a/tests/test_oauth/test_get_token.py +++ b/tests/test_oauth/test_get_token.py @@ -80,6 +80,6 @@ def test_if_got_token_no_redirect_required(self): with self.client.session_transaction() as sess: oauth.set_token( sess, PlanningProvider.oauth_token_name, "FRED") - response = self.client.get('/features/FT101') + response = self.client.get('/features/prov101...FT101') assert_equal(response.status_code, 200) assert_true("Amazing feature" in response.data) From a46948aee74587ea7c0b50ebae88d44e7fa0f8f1 Mon Sep 17 00:00:00 2001 From: Alex Couper Date: Tue, 7 May 2013 15:43:34 +0100 Subject: [PATCH 2/9] Add missing test module. --- tests/test_feature/test_lib.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 tests/test_feature/test_lib.py diff --git a/tests/test_feature/test_lib.py b/tests/test_feature/test_lib.py new file mode 100644 index 0000000..8a94f0a --- /dev/null +++ b/tests/test_feature/test_lib.py @@ -0,0 +1,22 @@ +#!/usr/bin/env python +#-*- coding: utf-8 -*- +from mock import Mock +from nose.tools import assert_equals + +from deploystream.providers.interfaces import IPlanningProvider +from deploystream.apps.feature.lib import get_all_features + +NON_ASCII_STRING = u"都بيببيðéáöþ" + + +def test_non_ascii_chars(): + mock_provider = Mock() + mock_provider.get_features.return_value = [{ + "project": NON_ASCII_STRING, + "id": NON_ASCII_STRING, + 'title': NON_ASCII_STRING + }] + + resp = get_all_features({IPlanningProvider: [mock_provider]}) + + assert_equals(resp[0].title, NON_ASCII_STRING) From 1533294dc9f4efce60b59b56f78acc1444274e58 Mon Sep 17 00:00:00 2001 From: Alex Couper Date: Tue, 7 May 2013 16:02:01 +0100 Subject: [PATCH 3/9] Look up id in only the relevant provider. --- deploystream/apps/feature/lib.py | 30 +++++++++++++----------------- deploystream/apps/feature/views.py | 8 ++++++-- deploystream/exceptions.py | 4 ++++ deploystream/providers/__init__.py | 2 ++ tests/test_feature/test_view.py | 10 ++++------ tests/test_oauth/test_get_token.py | 3 ++- 6 files changed, 31 insertions(+), 26 deletions(-) diff --git a/deploystream/apps/feature/lib.py b/deploystream/apps/feature/lib.py index d73f10d..03ac6ac 100644 --- a/deploystream/apps/feature/lib.py +++ b/deploystream/apps/feature/lib.py @@ -2,6 +2,7 @@ #-*- coding: utf-8 -*- from deploystream import app +from deploystream.exceptions import UnknownProviderException from deploystream.providers.interfaces import ( IBuildInfoProvider, IPlanningProvider, ISourceCodeControlProvider) from .models import Branch, BuildInfo, Feature @@ -35,26 +36,21 @@ def get_feature_info(feature_provider, feature_id, providers): :param providers: A dictionary of all providers. + + :raises: + UnknownProviderException - if no such name found. + """ - # TODO: since features may come from various origins, we need - # at this stage to either use a feature id that is a string such as - # "github:pretenders/deploystream:15" or to have additional arguments - # for provider and project. In any case we probably need providers to - # have an identifying string such as "github", "jira", "sprintly"... - - # First get feature info from the management providers - # This needs rewriting according to previous paragraph. For now: - # Only one management provider should know about this feature, - # so we stop on first success + # First get feature info from the management provider + feature = None - for provider in providers[IPlanningProvider]: - feature = Feature(provider, None, - **provider.get_feature_info(feature_id)) - if feature: - break + try: + planning_provider = providers['by_name'][feature_provider] + except KeyError: + raise UnknownProviderException(feature_provider) - if not feature: - return + feature = Feature(planning_provider, + **planning_provider.get_feature_info(feature_id)) # Then get any branch info from any source control providers for provider in providers[ISourceCodeControlProvider]: diff --git a/deploystream/apps/feature/views.py b/deploystream/apps/feature/views.py index 390e85c..1193ca6 100644 --- a/deploystream/apps/feature/views.py +++ b/deploystream/apps/feature/views.py @@ -1,11 +1,12 @@ from functools import wraps -from flask import json, Response +from flask import json, Response, abort from deploystream import app from deploystream.apps.feature.lib import get_feature_info, get_all_features from deploystream.lib.transforms import nativify from deploystream.decorators import needs_providers +from deploystream.exceptions import UnknownProviderException def as_json(func): @@ -34,5 +35,8 @@ def list_features(providers): @as_json def view_feature(provider_feature_id, providers): provider_id, feature_id = provider_feature_id.split('...') - feature = get_feature_info(provider_id, feature_id, providers) + try: + feature = get_feature_info(provider_id, feature_id, providers) + except UnknownProviderException: + abort(404) return feature diff --git a/deploystream/exceptions.py b/deploystream/exceptions.py index d920253..0f61630 100644 --- a/deploystream/exceptions.py +++ b/deploystream/exceptions.py @@ -2,3 +2,7 @@ class MissingTokenException(Exception): def __init__(self, missing_token, *args, **kwargs): super(Exception, self).__init__(*args, **kwargs) self.missing_token = missing_token + + +class UnknownProviderException(Exception): + pass diff --git a/deploystream/providers/__init__.py b/deploystream/providers/__init__.py index 159bb9c..2d4238e 100644 --- a/deploystream/providers/__init__.py +++ b/deploystream/providers/__init__.py @@ -33,6 +33,7 @@ def get_providers(configs, session): interface """ providers = defaultdict(list) + providers['by_name'] = {} for name, config in configs: provider_class = ALL_PROVIDER_CLASSES[name] kwargs = {} @@ -60,6 +61,7 @@ def get_providers(configs, session): ISourceCodeControlProvider]: if is_implementation(provider, iface): providers[iface].append(provider) + providers['by_name'][provider.name] = provider print("INFO: Initialised provider {0}".format(name)) except Exception: print("ERROR: Failed to initialise provider {0}: {1}" diff --git a/tests/test_feature/test_view.py b/tests/test_feature/test_view.py index ebcd4b3..dc165e0 100644 --- a/tests/test_feature/test_view.py +++ b/tests/test_feature/test_view.py @@ -29,6 +29,7 @@ def get_feature_info(self, feature_id, **kwargs): "url": "http://planning_site/{0}".format(feature_id), "feature_type": "story", "owner": "Bob", + "project": "P1", "description": "Too good for words..." } @@ -71,9 +72,6 @@ def test_feature_view_shows_details(self): {'testplan': PlanningProvider, 'testsource': SourceCodeProvider, 'testbuild': BuildInfoProvider}) - def test_only_uses_providers_user_specifies(self): - conf = deploystream.app.config - del conf['USER_SPECIFIC_INFO']['provider_config'][0] - - response = self.client.get('/features/plan...FT101') - assert "Amazing feature that will blow your mind" not in response.data + def test_returns_404_on_unknown_provider(self): + response = self.client.get('/features/planmissing...FT101') + assert 404 == response.status_code diff --git a/tests/test_oauth/test_get_token.py b/tests/test_oauth/test_get_token.py index 1c618a7..4aebb07 100644 --- a/tests/test_oauth/test_get_token.py +++ b/tests/test_oauth/test_get_token.py @@ -40,7 +40,8 @@ def get_feature_info(self, feature_id, **kwargs): "url": "http://planning_site/{0}".format(feature_id), "feature_type": "story", "owner": "Bob", - "description": "Too good for words..." + "description": "Too good for words...", + "project": "p2" } From 05ca666a368cbbefaf12635351085b931c4116f6 Mon Sep 17 00:00:00 2001 From: Alex Couper Date: Tue, 7 May 2013 17:35:43 +0100 Subject: [PATCH 4/9] Add hierarchy workings based on regex. --- deploystream/apps/feature/lib.py | 6 +-- deploystream/apps/feature/models.py | 8 +++- deploystream/lib/hierarchy.py | 52 +++++++++++++++++++++++ deploystream/providers/github/__init__.py | 21 ++++++++- tests/test_lib/test_hierarchy.py | 28 ++++++++++++ 5 files changed, 109 insertions(+), 6 deletions(-) create mode 100644 deploystream/lib/hierarchy.py create mode 100644 tests/test_lib/test_hierarchy.py diff --git a/deploystream/apps/feature/lib.py b/deploystream/apps/feature/lib.py index 03ac6ac..87b6f16 100644 --- a/deploystream/apps/feature/lib.py +++ b/deploystream/apps/feature/lib.py @@ -39,7 +39,6 @@ def get_feature_info(feature_provider, feature_id, providers): :raises: UnknownProviderException - if no such name found. - """ # First get feature info from the management provider @@ -54,12 +53,13 @@ def get_feature_info(feature_provider, feature_id, providers): # Then get any branch info from any source control providers for provider in providers[ISourceCodeControlProvider]: - for branch_data in provider.get_repo_branches_involved(feature_id): + for branch_data in provider.get_repo_branches_involved( + feature_id, app.config['HIERARCHY_REGEXES']): feature.add_branch(Branch(*branch_data, provider=provider)) # Use that branch info, along with configuration regexes to create a # hierarchy of the branches involved in the feature. - feature.create_hierarchy_trees(app.config['HIERARCHY_REGEXES']) + feature.create_hierarchy_trees() # Ask source control providers for merging information at this point. for provider in providers[ISourceCodeControlProvider]: diff --git a/deploystream/apps/feature/models.py b/deploystream/apps/feature/models.py index b4540e1..534c65d 100644 --- a/deploystream/apps/feature/models.py +++ b/deploystream/apps/feature/models.py @@ -41,7 +41,7 @@ def __init__(self, provider, project, id, title, self.branches = [] self.trees = [] - def create_hierarchy_trees(self, regexes): + def create_hierarchy_trees(self): "Create hierarchy trees - one for each repo." pass @@ -60,9 +60,12 @@ class Branch(object): ``branch_name`` - The name of the branch. ``latest_commit`` - The head commmit, or latest revision in this branch. + ``level`` - The level this Branch falls in the hierarchy for + the feature. ``provider`` - The provider instance that found this branch information. + Instances are eventually populated with these values: ``build_info`` - Build information for this particular branch. @@ -75,7 +78,7 @@ class Branch(object): or would have the same parent if one existed. """ - def __init__(self, repo_name, branch_name, latest_commit, provider): + def __init__(self, repo_name, branch_name, latest_commit, level, provider): self.parent = None self.children = [] self.siblings = [] # Will be needed in the cases where we have no @@ -84,6 +87,7 @@ def __init__(self, repo_name, branch_name, latest_commit, provider): self.repo_name = repo_name self.branch_name = branch_name self.latest_commit = latest_commit + self.level = level self._provider = provider diff --git a/deploystream/lib/hierarchy.py b/deploystream/lib/hierarchy.py new file mode 100644 index 0000000..e8aeccd --- /dev/null +++ b/deploystream/lib/hierarchy.py @@ -0,0 +1,52 @@ +import re + + +def create_single_regex(feature_id, hierarchical_regexes): + """ + Create a single regex to be used to find which level a branch is on. + + :param feature_id: + The id of the feature. This is substituted into the + ``hierarchical_regexes`` if they use {FEATURE_ID} anywhere. + + :param hierarchical_regexes: + A list of regexes to be joined into one single regex. + + :returns: + A single regex for easier matching. + """ + subs = [] + for index, regex in enumerate(hierarchical_regexes): + subs.append("(?P{1})".format(index, regex)) + full_regex = "|".join(subs) + full_regex = full_regex.format(FEATURE_ID=feature_id) + return full_regex + + +def match_with_levels(feature_id, branch, hierarchical_regexes): + """ + Filter and return the branches in appropriate levels. + + :param feature_id: + The feature to filter the branch names on. + + :param branches: + A list of branch names to filter. + + :param hierarchical_regexes: + A list of regexes assumed to be in descending order of branch status. + + :returns: + The positional index that the branch should be found it. Or None if it + does not match. + """ + regex = create_single_regex(feature_id, hierarchical_regexes) + + result = re.match(regex, branch) + if not result: + return None + + for level, match in result.groupdict().items(): + if match: + index = int(level.split('level_')[1]) + return index diff --git a/deploystream/providers/github/__init__.py b/deploystream/providers/github/__init__.py index 67255ec..590f4f8 100644 --- a/deploystream/providers/github/__init__.py +++ b/deploystream/providers/github/__init__.py @@ -1,8 +1,9 @@ import github3 +import re from zope import interface from deploystream.providers.interfaces import IPlanningProvider -from deploystream.lib import transforms +from deploystream.lib import transforms, hierarchy __all__ = ['GithubProvider'] @@ -85,3 +86,21 @@ def get_oauth_data(self): 'scope': 'repo' }, } + + def get_repo_branches_involved(self, feature_id, hierarchy_regexes): + branch_list = [] + + for repo in self.repositories: + for branch in repo.iter_branches(): + level = hierarchy.match_with_levels( + feature_id, branch, hierarchy_regexes) + if not level: + continue + branch_list.append({ + "repo_name": repo.name, + "branch_name": branch.name, + "latest_commit": branch.commit, + "level": level, + }) + + return branch_list diff --git a/tests/test_lib/test_hierarchy.py b/tests/test_lib/test_hierarchy.py new file mode 100644 index 0000000..25861fb --- /dev/null +++ b/tests/test_lib/test_hierarchy.py @@ -0,0 +1,28 @@ +from deploystream.lib.hierarchy import match_with_levels + +from nose.tools import assert_equals + + +def test_match_with_levels(): + "Test that we get back what we'd expect when matching branches" + branches_results = [ + ('master', 0), + ('develop', 1), + ('story/23', 2), + ('dev/23/alex', 3), + ('dev/23/carles', 3), + + ('story/45/alex', None), + ('dev/99/carles', None), + ] + regexes = [ + 'master', + 'develop', + 'story/{FEATURE_ID}(/[a-z]*)?', + 'dev/{FEATURE_ID}/[a-z]*', + '[a-zA-Z]*/{FEATURE_ID}/[a-zA-Z]*' + ] + + for branch, expected in branches_results: + result = match_with_levels('23', branch, regexes) + assert_equals(result, expected) From 34bb155212fbc4ab3e25276a0a304c28514a565b Mon Sep 17 00:00:00 2001 From: Alex Couper Date: Thu, 9 May 2013 17:52:26 +0100 Subject: [PATCH 5/9] Use a sensible URL and a flat dict. --- deploystream/apps/feature/lib.py | 10 ++++------ deploystream/apps/feature/models.py | 5 +++-- deploystream/apps/feature/views.py | 7 +++---- deploystream/lib/hierarchy.py | 2 +- deploystream/providers/__init__.py | 3 +-- tests/test_feature/test_view.py | 4 ++-- tests/test_oauth/test_get_token.py | 4 ++-- 7 files changed, 16 insertions(+), 19 deletions(-) diff --git a/deploystream/apps/feature/lib.py b/deploystream/apps/feature/lib.py index 87b6f16..4a05916 100644 --- a/deploystream/apps/feature/lib.py +++ b/deploystream/apps/feature/lib.py @@ -40,14 +40,12 @@ def get_feature_info(feature_provider, feature_id, providers): :raises: UnknownProviderException - if no such name found. """ - # First get feature info from the management provider - - feature = None - try: - planning_provider = providers['by_name'][feature_provider] - except KeyError: + if feature_provider not in providers: raise UnknownProviderException(feature_provider) + # First get feature info from the management provider + planning_provider = providers[feature_provider] + feature = Feature(planning_provider, **planning_provider.get_feature_info(feature_id)) diff --git a/deploystream/apps/feature/models.py b/deploystream/apps/feature/models.py index 534c65d..284f543 100644 --- a/deploystream/apps/feature/models.py +++ b/deploystream/apps/feature/models.py @@ -60,8 +60,9 @@ class Branch(object): ``branch_name`` - The name of the branch. ``latest_commit`` - The head commmit, or latest revision in this branch. - ``level`` - The level this Branch falls in the hierarchy for - the feature. + ``level`` - The numerical level that this branch falls in the + hierarchy for the feature - where 0 is the highest + level. ``provider`` - The provider instance that found this branch information. diff --git a/deploystream/apps/feature/views.py b/deploystream/apps/feature/views.py index 1193ca6..28c7bd0 100644 --- a/deploystream/apps/feature/views.py +++ b/deploystream/apps/feature/views.py @@ -30,13 +30,12 @@ def list_features(providers): return features -@app.route('/features/', methods=['GET']) +@app.route('/features//', methods=['GET']) @needs_providers @as_json -def view_feature(provider_feature_id, providers): - provider_id, feature_id = provider_feature_id.split('...') +def view_feature(source_id, feature_id, providers): try: - feature = get_feature_info(provider_id, feature_id, providers) + feature = get_feature_info(source_id, feature_id, providers) except UnknownProviderException: abort(404) return feature diff --git a/deploystream/lib/hierarchy.py b/deploystream/lib/hierarchy.py index e8aeccd..d3b0801 100644 --- a/deploystream/lib/hierarchy.py +++ b/deploystream/lib/hierarchy.py @@ -37,7 +37,7 @@ def match_with_levels(feature_id, branch, hierarchical_regexes): A list of regexes assumed to be in descending order of branch status. :returns: - The positional index that the branch should be found it. Or None if it + The positional index that the branch should be found in. Or None if it does not match. """ regex = create_single_regex(feature_id, hierarchical_regexes) diff --git a/deploystream/providers/__init__.py b/deploystream/providers/__init__.py index 2d4238e..f65cfab 100644 --- a/deploystream/providers/__init__.py +++ b/deploystream/providers/__init__.py @@ -33,7 +33,6 @@ def get_providers(configs, session): interface """ providers = defaultdict(list) - providers['by_name'] = {} for name, config in configs: provider_class = ALL_PROVIDER_CLASSES[name] kwargs = {} @@ -61,7 +60,7 @@ def get_providers(configs, session): ISourceCodeControlProvider]: if is_implementation(provider, iface): providers[iface].append(provider) - providers['by_name'][provider.name] = provider + providers[provider.name] = provider print("INFO: Initialised provider {0}".format(name)) except Exception: print("ERROR: Failed to initialise provider {0}: {1}" diff --git a/tests/test_feature/test_view.py b/tests/test_feature/test_view.py index dc165e0..363331d 100644 --- a/tests/test_feature/test_view.py +++ b/tests/test_feature/test_view.py @@ -65,7 +65,7 @@ def setUp(self): 'testsource': SourceCodeProvider, 'testbuild': BuildInfoProvider}) def test_feature_view_shows_details(self): - response = self.client.get('/features/plan...FT101') + response = self.client.get('/features/plan/FT101') assert "Amazing feature that will blow your mind" in response.data @patch("deploystream.providers.ALL_PROVIDER_CLASSES", @@ -73,5 +73,5 @@ def test_feature_view_shows_details(self): 'testsource': SourceCodeProvider, 'testbuild': BuildInfoProvider}) def test_returns_404_on_unknown_provider(self): - response = self.client.get('/features/planmissing...FT101') + response = self.client.get('/features/planmissing/FT101') assert 404 == response.status_code diff --git a/tests/test_oauth/test_get_token.py b/tests/test_oauth/test_get_token.py index 4aebb07..03b7499 100644 --- a/tests/test_oauth/test_get_token.py +++ b/tests/test_oauth/test_get_token.py @@ -69,7 +69,7 @@ def test_providers_requiring_oauth_token_force_redirect(self): configure_oauth_routes(deploystream.providers.ALL_PROVIDER_CLASSES) - response = self.client.get('/features/FT101') + response = self.client.get('/features/prov101/FT101') assert_equal(response.status_code, 302) assert_true("http://auth_url" in response.location) @@ -81,6 +81,6 @@ def test_if_got_token_no_redirect_required(self): with self.client.session_transaction() as sess: oauth.set_token( sess, PlanningProvider.oauth_token_name, "FRED") - response = self.client.get('/features/prov101...FT101') + response = self.client.get('/features/prov101/FT101') assert_equal(response.status_code, 200) assert_true("Amazing feature" in response.data) From 89fabfa2e4707d0ebf2accadd658e23a315217e4 Mon Sep 17 00:00:00 2001 From: Alex Couper Date: Mon, 13 May 2013 10:48:47 +0100 Subject: [PATCH 6/9] Add test of get_repo_branches_involved. --- deploystream/providers/github/__init__.py | 8 ++- tests/test_providers/test_github_provider.py | 53 +++++++++++++++++++- 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/deploystream/providers/github/__init__.py b/deploystream/providers/github/__init__.py index 590f4f8..b7968fa 100644 --- a/deploystream/providers/github/__init__.py +++ b/deploystream/providers/github/__init__.py @@ -36,6 +36,10 @@ def __init__(self, token, organization=None, **kwargs): user's issues will be tracked. """ self.github = github3.login(token=token) + + if "github_url" in kwargs: + self.github._github_url = kwargs['github_url'] + if not organization: self.repositories = list(self.github.iter_repos()) else: @@ -93,8 +97,8 @@ def get_repo_branches_involved(self, feature_id, hierarchy_regexes): for repo in self.repositories: for branch in repo.iter_branches(): level = hierarchy.match_with_levels( - feature_id, branch, hierarchy_regexes) - if not level: + feature_id, branch.name, hierarchy_regexes) + if level is None: continue branch_list.append({ "repo_name": repo.name, diff --git a/tests/test_providers/test_github_provider.py b/tests/test_providers/test_github_provider.py index 3fb509f..4dfdf48 100644 --- a/tests/test_providers/test_github_provider.py +++ b/tests/test_providers/test_github_provider.py @@ -6,10 +6,10 @@ IPlanningProvider, IOAuthProvider, is_implementation) -@patch('deploystream.providers.github.github3') -def test_get_features(github3): +def mock_github3(github3): mock_repo = Mock() mock_repo.has_issues = True + mock_repo.name = 'repo_1' issue1 = { 'title': 'Hello', @@ -31,9 +31,33 @@ def test_get_features(github3): mock_repo.iter_issues.return_value = [ mock_issue1, mock_issue2 ] + + branch1 = { + 'name': 'master', + 'commit': 'CoMmItHaShMaStEr', + } + branch2 = { + 'name': 'story/5/alex', + 'commit': 'CoMmItHaSh5', + } + branch3 = { + 'name': 'story/23/alex', + 'commit': 'CoMmItHaSh23', + } + mock_branch1, mock_branch2, mock_branch3 = Mock(), Mock(), Mock() + mock_branch1.__dict__ = branch1 + mock_branch2.__dict__ = branch2 + mock_branch3.__dict__ = branch3 + mock_repo.iter_branches.return_value = [ + mock_branch1, mock_branch2, mock_branch3 + ] github3.login.return_value = github3 github3.iter_repos.return_value = [mock_repo] + +@patch('deploystream.providers.github.github3') +def test_get_features(github3): + mock_github3(github3) github_provider = GithubProvider('token') features = github_provider.get_features() @@ -50,3 +74,28 @@ def test_get_features(github3): def test_implements_expected_interfaces(_): assert_true(is_implementation(GithubProvider('token'), IPlanningProvider)) assert_true(is_implementation(GithubProvider('token'), IOAuthProvider)) + + +@patch('deploystream.providers.github.github3') +def test_get_repo_branches_involved(github3): + mock_github3(github3) + github_provider = GithubProvider('token') + branches = github_provider.get_repo_branches_involved(5, + hierarchy_regexes=[ + 'master', + 'develop', + 'story/{FEATURE_ID}(/[a-z]*)?', + 'dev/{FEATURE_ID}/[a-z]*', + '[a-zA-Z]*/{FEATURE_ID}/[a-zA-Z]*' + ]) + assert_equal(2, len(branches)) + assert_true({ + "repo_name": "repo_1", + "branch_name": "master", + "latest_commit": 'CoMmItHaShMaStEr', + "level": 0} in branches) + assert_true({ + "repo_name": "repo_1", + "branch_name": "story/5/alex", + "latest_commit": "CoMmItHaSh5", + "level": 2} in branches) From 1d7035d35c61415fa493890f2498cbf5dc7565db Mon Sep 17 00:00:00 2001 From: Alex Couper Date: Mon, 13 May 2013 11:45:07 +0100 Subject: [PATCH 7/9] Add integration test. --- config/local_settings_test.py | 8 +---- deploystream/lib/hierarchy.py | 2 +- deploystream/providers/github/__init__.py | 34 ++++++++++++++------ requirements/test.txt | 1 + tests/__init__.py | 9 ++++++ tests/integration/__init__.py | 0 tests/integration/test_github.py | 32 ++++++++++++++++++ tests/test_lib/test_hierarchy.py | 16 ++++----- tests/test_providers/test_git_provider.py | 5 ++- tests/test_providers/test_github_provider.py | 19 ++++------- 10 files changed, 85 insertions(+), 41 deletions(-) create mode 100644 tests/integration/__init__.py create mode 100644 tests/integration/test_github.py diff --git a/config/local_settings_test.py b/config/local_settings_test.py index 1521a02..2c8b45f 100644 --- a/config/local_settings_test.py +++ b/config/local_settings_test.py @@ -1,11 +1,5 @@ GITHUB_CONFIG = { - 'repositories': [ - ('pretenders', 'deploystream'), - ('pretenders', 'pretenders'), - ('txels', 'autojenkins'), - ('txels', 'ddt'), - ('txels', 'apitopy'), - ], + 'organization': 'pretenders', } SPRINTLY_CONFIG = { diff --git a/deploystream/lib/hierarchy.py b/deploystream/lib/hierarchy.py index d3b0801..611a5ce 100644 --- a/deploystream/lib/hierarchy.py +++ b/deploystream/lib/hierarchy.py @@ -17,7 +17,7 @@ def create_single_regex(feature_id, hierarchical_regexes): """ subs = [] for index, regex in enumerate(hierarchical_regexes): - subs.append("(?P{1})".format(index, regex)) + subs.append("(?P^{1}$)".format(index, regex)) full_regex = "|".join(subs) full_regex = full_regex.format(FEATURE_ID=feature_id) return full_regex diff --git a/deploystream/providers/github/__init__.py b/deploystream/providers/github/__init__.py index b7968fa..3396e42 100644 --- a/deploystream/providers/github/__init__.py +++ b/deploystream/providers/github/__init__.py @@ -26,25 +26,39 @@ class GithubProvider(object): name = 'github' oauth_token_name = name - def __init__(self, token, organization=None, **kwargs): + def __init__(self, token, organization=None, repositories=None, **kwargs): """ Initialise the provider by giving it GitHub credentials and repos. :param organization: The name of the organization who's repository issues should be - identified in GitHub. If ``None`` then the authenticated - user's issues will be tracked. - """ - self.github = github3.login(token=token) + identified in GitHub. If ``None`` and no ``repositories`` given, + then the authenticated user's issues will be tracked. - if "github_url" in kwargs: - self.github._github_url = kwargs['github_url'] + :param repositories: + A list of tuples containing (, ) that identify + a repository in GitHub. This is only looked at if ``organization`` + is ``None``. + """ - if not organization: - self.repositories = list(self.github.iter_repos()) + if token is None and "username" in kwargs and "password" in kwargs: + # We can login using username and password for testing purposes + self.github = github3.login( + kwargs['username'], + password=kwargs['password'] + ) else: + self.github = github3.login(token=token) + + if organization: org = self.github.organization(organization) self.repositories = list(org.iter_repos()) + elif repositories: + self.repositories = [] + for owner, repo in repositories: + self.repositories.append(self.github.repository(owner, repo)) + else: + self.repositories = list(self.github.iter_repos()) def get_features(self, **filters): """ @@ -103,7 +117,7 @@ def get_repo_branches_involved(self, feature_id, hierarchy_regexes): branch_list.append({ "repo_name": repo.name, "branch_name": branch.name, - "latest_commit": branch.commit, + "latest_commit": branch.commit.sha, "level": level, }) diff --git a/requirements/test.txt b/requirements/test.txt index 87ac6eb..2025fe5 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -7,3 +7,4 @@ pyhamcrest # matchers for use in test assertions nodeenv # For creating an isolated node/npm environment inside the # python virtualenv, to install node.js packages +supermutes # To dotify dictionaries. diff --git a/tests/__init__.py b/tests/__init__.py index 0491cd1..4deb8f3 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -7,3 +7,12 @@ def load_fixture(filename): with file(os.path.join(TEST_DATA, filename)) as f: contents = f.read() return contents + + +DEFAULT_HIERARCHY_REGEXES = [ + 'master', + 'develop', + 'story/{FEATURE_ID}(/[a-z]*)?', + 'dev/{FEATURE_ID}/[a-z]*', + '[a-zA-Z]*/{FEATURE_ID}/[a-zA-Z]*' +] diff --git a/tests/integration/__init__.py b/tests/integration/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/integration/test_github.py b/tests/integration/test_github.py new file mode 100644 index 0000000..a86fe29 --- /dev/null +++ b/tests/integration/test_github.py @@ -0,0 +1,32 @@ +from mock import Mock, patch +from nose.tools import assert_equal, assert_true + +from deploystream.providers.github import GithubProvider +from deploystream.providers.interfaces import ( + IPlanningProvider, IOAuthProvider, is_implementation) +from tests import DEFAULT_HIERARCHY_REGEXES +from deploystream import app + + +def test_get_repo_branches_involved(): + "Test ``get_repo_branches_involved`` using ``pretenders/dummyrepo`` repo." + github_provider = GithubProvider( + token=None, + username=app.config['GITHUB_CONFIG']['username'], + password=app.config['GITHUB_CONFIG']['password'], + repositories=[('pretenders', 'dummyrepo')] + ) + branches = github_provider.get_repo_branches_involved(101, + hierarchy_regexes=DEFAULT_HIERARCHY_REGEXES) + + assert_equal(2, len(branches)) + assert_true({ + "repo_name": "dummyrepo", + "branch_name": "master", + "latest_commit": '0f6eefefc14f362a2c6f804df69aa83bac48c20b', + "level": 0} in branches) + assert_true({ + "repo_name": "dummyrepo", + "branch_name": "story/101/fred", + "latest_commit": "0f6eefefc14f362a2c6f804df69aa83bac48c20b", + "level": 2} in branches) diff --git a/tests/test_lib/test_hierarchy.py b/tests/test_lib/test_hierarchy.py index 25861fb..cacd50f 100644 --- a/tests/test_lib/test_hierarchy.py +++ b/tests/test_lib/test_hierarchy.py @@ -1,7 +1,8 @@ -from deploystream.lib.hierarchy import match_with_levels - from nose.tools import assert_equals +from deploystream.lib.hierarchy import match_with_levels +from tests import DEFAULT_HIERARCHY_REGEXES + def test_match_with_levels(): "Test that we get back what we'd expect when matching branches" @@ -12,17 +13,12 @@ def test_match_with_levels(): ('dev/23/alex', 3), ('dev/23/carles', 3), + ('somestory/234/carles', None), + ('story/234/carles', None), ('story/45/alex', None), ('dev/99/carles', None), ] - regexes = [ - 'master', - 'develop', - 'story/{FEATURE_ID}(/[a-z]*)?', - 'dev/{FEATURE_ID}/[a-z]*', - '[a-zA-Z]*/{FEATURE_ID}/[a-zA-Z]*' - ] for branch, expected in branches_results: - result = match_with_levels('23', branch, regexes) + result = match_with_levels('23', branch, DEFAULT_HIERARCHY_REGEXES) assert_equals(result, expected) diff --git a/tests/test_providers/test_git_provider.py b/tests/test_providers/test_git_provider.py index 116b02f..c38e765 100644 --- a/tests/test_providers/test_git_provider.py +++ b/tests/test_providers/test_git_provider.py @@ -19,7 +19,10 @@ def ensure_dummy_clone_available(): os.system('git clone git://github.com/pretenders/dummyrepo.git {0}' .format(folder_name)) else: - os.system('git --git-dir={0} fetch'.format(folder_name)) + cmd = 'git --git-dir={0}/.git fetch'.format(folder_name) + ans = os.system(cmd) + if ans != 0: + raise Exception("Git fetch failed") @with_setup(ensure_dummy_clone_available) diff --git a/tests/test_providers/test_github_provider.py b/tests/test_providers/test_github_provider.py index 4dfdf48..f337802 100644 --- a/tests/test_providers/test_github_provider.py +++ b/tests/test_providers/test_github_provider.py @@ -4,6 +4,7 @@ from deploystream.providers.github import GithubProvider from deploystream.providers.interfaces import ( IPlanningProvider, IOAuthProvider, is_implementation) +from tests import DEFAULT_HIERARCHY_REGEXES def mock_github3(github3): @@ -34,15 +35,15 @@ def mock_github3(github3): branch1 = { 'name': 'master', - 'commit': 'CoMmItHaShMaStEr', + 'commit': Mock(sha='CoMmItHaSh-MaStEr'), } branch2 = { 'name': 'story/5/alex', - 'commit': 'CoMmItHaSh5', + 'commit': Mock(sha='CoMmItHaSh-5'), } branch3 = { 'name': 'story/23/alex', - 'commit': 'CoMmItHaSh23', + 'commit': Mock(sha='CoMmItHaSh-23'), } mock_branch1, mock_branch2, mock_branch3 = Mock(), Mock(), Mock() mock_branch1.__dict__ = branch1 @@ -81,21 +82,15 @@ def test_get_repo_branches_involved(github3): mock_github3(github3) github_provider = GithubProvider('token') branches = github_provider.get_repo_branches_involved(5, - hierarchy_regexes=[ - 'master', - 'develop', - 'story/{FEATURE_ID}(/[a-z]*)?', - 'dev/{FEATURE_ID}/[a-z]*', - '[a-zA-Z]*/{FEATURE_ID}/[a-zA-Z]*' - ]) + hierarchy_regexes=DEFAULT_HIERARCHY_REGEXES) assert_equal(2, len(branches)) assert_true({ "repo_name": "repo_1", "branch_name": "master", - "latest_commit": 'CoMmItHaShMaStEr', + "latest_commit": 'CoMmItHaSh-MaStEr', "level": 0} in branches) assert_true({ "repo_name": "repo_1", "branch_name": "story/5/alex", - "latest_commit": "CoMmItHaSh5", + "latest_commit": "CoMmItHaSh-5", "level": 2} in branches) From 78c40277ee42a33432c7f32fb2e2c495a5916743 Mon Sep 17 00:00:00 2001 From: Alex Couper Date: Mon, 13 May 2013 11:50:44 +0100 Subject: [PATCH 8/9] Change test to import uname and password for testing purposes. --- config/local_settings_test.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/config/local_settings_test.py b/config/local_settings_test.py index 2c8b45f..f1ed5dd 100644 --- a/config/local_settings_test.py +++ b/config/local_settings_test.py @@ -1,5 +1,5 @@ GITHUB_CONFIG = { - 'organization': 'pretenders', + 'organization': 'pretenders' } SPRINTLY_CONFIG = { @@ -13,3 +13,14 @@ GIT_CONFIG = { } + +try: + from non_github_settings import GITHUB_USERNAME, GITHUB_PASSWORD + GITHUB_CONFIG['username'] = GITHUB_USERNAME + GITHUB_CONFIG['password'] = GITHUB_PASSWORD +except ImportError: + print ("Failed to import from non_github_settings. \n" + "You need GITHUB_PASSWORD and GITHUB_USERNAME defined in a module " + "named ``non_github_settings`` in order to run the tests." + ) + raise From 5719681c8635c062cb75e7f19792180ea9fe0e6b Mon Sep 17 00:00:00 2001 From: Alex Couper Date: Mon, 13 May 2013 11:59:46 +0100 Subject: [PATCH 9/9] Update test.txt requirements. --- requirements/test.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/requirements/test.txt b/requirements/test.txt index 2025fe5..87ac6eb 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -7,4 +7,3 @@ pyhamcrest # matchers for use in test assertions nodeenv # For creating an isolated node/npm environment inside the # python virtualenv, to install node.js packages -supermutes # To dotify dictionaries.