diff --git a/.ci/travis/script.sh b/.ci/travis/script.sh index 593c61d..8540269 100755 --- a/.ci/travis/script.sh +++ b/.ci/travis/script.sh @@ -8,8 +8,9 @@ if [[ $PYLINT == "1" ]]; then # more noisy checks that don't effect quality for testing # purposes. pylint tests \ - --disable missing-docstring,invalid-name \ - --disable protected-access,no-self-use,unused-argument + --disable missing-docstring,invalid-name,too-many-arguments \ + --disable protected-access,no-self-use,unused-argument \ + --disable too-few-public-methods fi if [[ $READTHEDOCS == "1" ]]; then diff --git a/pywincffi/dev/release.py b/pywincffi/dev/release.py index 48c695d..ce55de4 100644 --- a/pywincffi/dev/release.py +++ b/pywincffi/dev/release.py @@ -193,6 +193,9 @@ def download(cls, url, path=None, chunk_size=1024): return path +Issue = namedtuple( + "Issue", ("issue", "closed", "labels", "type", "number", "url", "title")) + class GitHubAPI(object): # pylint: disable=too-many-instance-attributes """ @@ -203,7 +206,11 @@ class GitHubAPI(object): # pylint: disable=too-many-instance-attributes PROJECT = "pywincffi" REPO_NAME = "opalmer/%s" % PROJECT - def __init__(self, version, branch="master"): + # NOTE: `repo` is for testing purposes. + def __init__(self, version, branch=None, repo_=None): + if branch is None: + branch = "master" + self.version = version self.branch = branch self.read_the_docs = \ @@ -221,9 +228,12 @@ def __init__(self, version, branch="master"): "pywincffi.github_token is not set in the config") self.hub = Github(login_or_token=github_token) - self.repo = self.hub.get_repo(self.REPO_NAME) + self.repo = repo_ + + if repo_ is None: # pragma: no cover + self.repo = self.hub.get_repo(self.REPO_NAME) - for milestone in self.repo.get_milestones(): + for milestone in self.repo.get_milestones(state="all"): if milestone.title == self.version: self.milestone = milestone break @@ -236,6 +246,46 @@ def commit(self): branch = self.repo.get_branch(self.branch) return branch.commit.sha + def issues(self): + """ + Generator which yields all issues associated with the milestone as well + as some extra information + """ + issues = self.repo.get_issues(milestone=self.milestone, state="all") + + for issue in issues: + # Determine the type of issue this is. This is mostly used + # when building the release message. + labels = set(label.name for label in issue.labels) + if "in progress" in labels or "in review" in labels: + logger.warning( + "Issue %s is still a work in progress", issue.number) + + if "bug" in labels: + issue_type = "bugs" + + elif "enhancement" in labels: + issue_type = "enhancements" + + elif "documentation" in labels: + issue_type = "documentation" + + elif "unittest" in labels: + issue_type = "unittests" + + else: + issue_type = "other" + + yield Issue( + issue=issue, + closed=issue.state == "closed", + labels=labels, + type=issue_type, + number=issue.number, + url=issue.html_url, + title=issue.title + ) + def release_message(self): """Produces release message for :meth:`create_release` to use.""" output = StringIO() @@ -252,46 +302,36 @@ def release_message(self): print("Pull requests and issues associated with this release.", file=output) print("", file=output) - issues = { - "bugs": [], - "enhancements": [], - "unittests": [], - "documentation": [], - "other": [] - } - for issue in self.repo.get_issues( - milestone=self.milestone, state="all"): - for label in issue.labels: - if label.name == "bug": - issues["bugs"].append(issue) - break - if label.name == "enhancement": - issues["enhancements"].append(issue) - break - if label.name == "documentation": - issues["documentation"].append(issue) - break - if label.name == "unittest": - issues["unittests"].append(issue) - break - else: - issues["other"].append(issue) - for value in issues.values(): - value.reverse() + issues = {} + for issue in self.issues(): + issues_by_type = issues.setdefault(issue.type, []) + issues_by_type.append(issue) + + for issues_list in issues.values(): + issues_list.sort(key=lambda item: item.number) for name in ( "enhancements", "bugs", "documentation", "unittests", "other"): - if issues[name]: + issues_for_name = issues.pop(name, []) + + if issues_for_name: + print("", file=output) print("#### %s" % name.title(), file=output) - for issue in issues[name.lower()]: - if issue.state != "closed": - logger.warning("Issue %s is not closed!", issue.number) + for issue in issues_for_name: print( - "[%s](%s) - %s" % ( - issue.number, issue.url, issue.title), + "[{number}]({url}) - {title}".format( + number=issue.number, url=issue.url, + title=issue.title + ), file=output) + # The for loop above establishes the order we want to see + # the various types of issues in. If a new type is added, + # we'll need to update this. + if issues: + raise ValueError("Unhandled keys: %s" % issues.keys()) + return output.getvalue() def create_release( @@ -309,14 +349,17 @@ def create_release( for release in self.repo.get_releases(): if release.tag_name == self.version: - if recreate: + if recreate and not dry_run: logger.warning( "Deleting existing release for %s", release.tag_name) release.delete_release() # TODO: make sure we delete the tag too else: - raise RuntimeError( - "A release for %r already exists" % self.version) + message = "A release for %r already exists" % self.version + if dry_run: + logger.error(message) + else: + raise RuntimeError(message) logger.info("Creating **draft** release %r", self.version) message = self.release_message() diff --git a/tests/test_dev/test_release.py b/tests/test_dev/test_release.py index a936bde..17c4618 100644 --- a/tests/test_dev/test_release.py +++ b/tests/test_dev/test_release.py @@ -6,8 +6,7 @@ import subprocess import sys from collections import namedtuple -from random import randint, choice -from textwrap import dedent +from random import randint, choice, shuffle from os.path import dirname, abspath, isfile, join, isdir, basename try: @@ -17,16 +16,21 @@ from httplib import OK, BAD_REQUEST from mock import Mock, patch -from github import Github from requests.adapters import HTTPAdapter from pywincffi.core.config import config from pywincffi.dev import release # used to mock top level functions from pywincffi.dev.release import ( - Session, AppVeyor, AppVeyorArtifact, GitHubAPI, check_wheel, docs_built) + Session, AppVeyor, AppVeyorArtifact, GitHubAPI, Issue, + check_wheel, docs_built) from pywincffi.dev.testutil import TestCase +def random_string(): + return "".join( + [choice(string.ascii_letters) for _ in range(randint(5, 20))]) + + class TestWheel(TestCase): """ Tests for constants of :func:`pywincffi.dev.release.test_wheel` @@ -129,12 +133,12 @@ class TestAppVeyor(TestCase): """ def setUp(self): super(TestAppVeyor, self).setUp() - self.job_id = self.random_string() + self.job_id = random_string() self.artifact_url = None self.artifact_path = None self.branch = { "build": { - "message": self.random_string(), + "message": random_string(), "jobs": [ { "jobId": self.job_id, @@ -147,12 +151,8 @@ def setUp(self): with patch.object(Session, "json", return_value=self.branch): self.appveyor = AppVeyor() - def random_string(self): - return "".join( - [choice(string.ascii_letters) for _ in range(randint(5, 20))]) - def test_creates_directory(self): - path = join(tempfile.gettempdir(), self.random_string()) + path = join(tempfile.gettempdir(), random_string()) with patch.object(Session, "json", return_value=[]): list(self.appveyor.artifacts(directory=path)) @@ -250,19 +250,33 @@ def setUp(self): super(GitHubAPICase, self).setUp() self.version = "0.0.0" - # The test token + # The test token. This ensures that we're never going to + # use a valid token which could cause problems on GitHub if + # used. self.token = "fake_token" github_token = config.get("pywincffi", "github_token") config.set("pywincffi", "github_token", self.token) self.addCleanup(config.set, "pywincffi", "github_token", github_token) - # Mocks for the Github class so we don't make any API calls - self.mocked_get_repo = patch.object( - Github, "get_repo", - return_value=Mock( - get_milestones=lambda: [Mock(title=self.version)])) - self.mocked_get_repo.start() - self.addCleanup(self.mocked_get_repo.stop) + # NOTE: `branch` should match the default + def api(self, version=None, branch=None, repo=None, milestones=None, + releases=None): + if version is None: + version = self.version + + if milestones is None: + milestones = [Mock(title=self.version)] + + if releases is None: + releases = [Mock(tag_name=self.version)] + + if repo is None: + repo = Mock( + get_milestones=Mock(return_value=milestones), + get_releases=Mock(return_value=releases) + ) + + return GitHubAPI(version, branch=branch, repo_=repo) class TestGitHubAPIInit(GitHubAPICase): @@ -270,33 +284,31 @@ class TestGitHubAPIInit(GitHubAPICase): Tests for :meth:`pywincffi.dev.release.GitHubAPI.__init__` """ def test_version(self): - api = GitHubAPI(self.version) + api = self.api(version=self.version) self.assertEqual(api.version, self.version) def test_branch_default(self): - api = GitHubAPI(self.version) + api = self.api() self.assertEqual(api.branch, "master") def test_branch_non_default(self): - api = GitHubAPI(self.version, branch="foobar") + api = self.api(version=self.version, branch="foobar") self.assertEqual(api.branch, "foobar") def test_token_not_set(self): config.set("pywincffi", "github_token", "") with self.assertRaises(RuntimeError): - GitHubAPI(self.version) + self.api() def test_milestone_not_found(self): - self._cleanups.remove((self.mocked_get_repo.stop, (), {})) - self.mocked_get_repo.stop() - mock = patch.object( - Github, "get_repo", - return_value=Mock( - get_milestones=lambda: [Mock(title="x.x.x")])) - mock.start() - self.addCleanup(mock.stop) with self.assertRaises(ValueError): - GitHubAPI(self.version) + self.api(version=self.version, milestones=[]) + + def test_looks_for_milestones_with_all_states(self): + api = self.api() + + # pylint: disable=no-member + api.repo.get_milestones.assert_called_with(state="all") class TestGitHubAPICommit(GitHubAPICase): @@ -304,133 +316,229 @@ class TestGitHubAPICommit(GitHubAPICase): Tests for :meth:`pywincffi.dev.release.GitHubAPI.commit` """ def test_commit(self): - api = GitHubAPI(self.version) expected = "da39a3ee5e6b4b0d3255bfef95601890afd80709" - with patch.object( - api.repo, "get_branch", return_value=Mock( - commit=Mock(sha=expected))): - self.assertEqual(api.commit(), expected) + api = self.api() + api.repo.get_branch.return_value = Mock(commit=Mock(sha=expected)) + self.assertEqual(api.commit(), expected) + + # pylint: disable=no-member + api.repo.get_branch.assert_called_with(api.branch) + +FakeLabel = namedtuple("FakeLabel", ("name", )) + + +class FakeIssue(object): + def __init__(self, type_=None, state=None, labels=None): + if type_ is None: + type_ = choice(["pull", "issue"]) + if state is None: + state = choice(["closed", "open"]) + + self.type_ = type_ + self.number = randint(1, 1024) + self.title = random_string() + self.labels = [] + self.state = state + + if labels is None: + for _ in range(randint(1, 5)): + self.labels.append( + FakeLabel(choice( + ["bug", "enhancement", "unittest", "documentation"]))) + else: + for label in labels: + self.labels.append(FakeLabel(label)) + + @property + def html_url(self): + if self.type_ == "pull": + url = "https://github.com/opalmer/pywincffi/pull/{number}" + else: + url = "https://github.com/opalmer/pywincffi/issues/{number}" + return url.format(number=self.number) + + +class GitHubAPICaseWithIssues(GitHubAPICase): + # pylint: disable=arguments-differ + def api(self, version=None, branch=None, repo=None, milestones=None, + issues=None, releases=None): + api = super(GitHubAPICaseWithIssues, self).api( + version=version, branch=branch, repo=repo, milestones=milestones, + releases=releases) + default_issues = [ + FakeIssue(), FakeIssue(), FakeIssue(), + FakeIssue(), FakeIssue(), FakeIssue() + ] + if issues is not None: + default_issues.extend(issues) + + shuffle(default_issues) + api.repo.get_issues = Mock(return_value=default_issues) + return api -class TestGitHubAPIReleaseMessage(GitHubAPICase): +class TestGitHubAPIIssues(GitHubAPICaseWithIssues): """ - Tests for :meth:`pywincffi.dev.release.GitHubAPI.release_message` + Tests for :meth:`pywincffi.dev.release.GitHubAPI.issues` """ - def test_gets_all_issues(self): - api = GitHubAPI(self.version) - - with patch.object(api.repo, "get_issues", return_value=[]) as mocked: - api.release_message() - - mocked.assert_called_with(milestone=api.milestone, state="all") - - def test_message(self): - label = namedtuple("Label", ("name", )) - - issues = [ - Mock(number=1, url="/1", title="Issue 1", state="closed", - labels=[label(name="unittest")]), - Mock(number=3, url="/3", title="Issue 3", state="closed", - labels=[label(name="enhancement")]), - Mock(number=2, url="/2", title="Issue 2", state="closed", - labels=[label(name="enhancement")]), - Mock(number=4, url="/4", title="Issue 4", state="closed", - labels=[label(name="bug")]), - Mock(number=5, url="/5", title="Issue 5", state="closed", - labels=[label(name="enhancement"), label(name="bug")]), - Mock(number=6, url="/6", title="Issue 6", state="closed", - labels=[]), - Mock(number=7, url="/7", title="Issue 7", state="closed", - labels=[label(name="documentation")]) - ] + def test_get_issues_keywords(self): + api = self.api() + list(api.issues()) + + # pylint: disable=no-member + api.repo.get_issues.assert_called_with( + milestone=api.milestone, state="all") + + def test_return_type(self): + api = self.api() + self.assertIsInstance(list(api.issues()), list) + + def test_return_type_value(self): + api = self.api() + for issue in api.issues(): + self.assertIsInstance(issue, Issue) + + def test_issue(self): + api = self.api() + + for issue in api.issues(): + self.assertIsInstance(issue.issue, FakeIssue) + + def test_closed(self): + api = self.api(issues=[FakeIssue(state="closed")]) + + for issue in api.issues(): + self.assertEqual(issue.closed, issue.issue.state == "closed") + + def issue_type(self): + api = self.api(issues=[ + FakeIssue(labels=["enhancement"]), FakeIssue(labels=["bug"]) + ]) + + for issue in api.issues(): + if "bug" in issue.labels: + expected_issue_type = "bugs" + + elif "enhancement" in issue.labels: + expected_issue_type = "enhancements" - api = GitHubAPI(self.version) - - with patch.object(api.repo, "get_issues", return_value=issues): - self.assertEqual(api.release_message().strip(), dedent(""" - ## External Links - Links for documentation, release files and other useful information. - * [Documentation](%s) - * [PyPi Package](%s) - * [GitHub Issues](%s) - - ## Pull Requests and Issues - Pull requests and issues associated with this release. - - #### Enhancements - [5](/5) - Issue 5 - [2](/2) - Issue 2 - [3](/3) - Issue 3 - #### Bugs - [4](/4) - Issue 4 - #### Documentation - [7](/7) - Issue 7 - #### Unittests - [1](/1) - Issue 1 - #### Other - [6](/6) - Issue 6 - """).strip() % ( - api.read_the_docs, api.pypi_release, api.milestone_filter)) - - -class TestGitHubAPICreateRelease(GitHubAPICase): + elif "documentation" in issue.labels: + expected_issue_type = "documentation" + + elif "unittest" in issue.labels: + expected_issue_type = "unittests" + + else: + expected_issue_type = "other" + + self.assertEqual(issue.type, expected_issue_type) + + def test_labels(self): + api = self.api(issues=[FakeIssue(labels=["foo"])]) + + for issue in api.issues(): + self.assertEqual( + issue.labels, + set(label.name for label in issue.issue.labels)) + + def test_issue_url(self): + api = self.api() + + for issue in api.issues(): + self.assertEqual(issue.issue.html_url, issue.url) + + +class TestGitHubAPIReleaseMessage(GitHubAPICaseWithIssues): + """ + Tests for :meth:`pywincffi.dev.release.GitHubAPI.release_message` + + .. note:: + + This is not a full test because most of the testing is completed + in TestGitHubAPIIssues() above. + """ + def test_return_type(self): + api = self.api() + self.assertIsInstance(api.release_message(), str) + + def test_doc_link(self): + api = self.api(releases=[]) + self.assertIn( + "* [Documentation](%s)" % api.read_the_docs, + api.release_message()) + + def test_pypi_link(self): + api = self.api() + self.assertIn( + "* [PyPi Package](%s)" % api.pypi_release, + api.release_message()) + + def test_github_issues(self): + api = self.api() + self.assertIn( + "* [GitHub Issues](%s)" % api.milestone_filter, + api.release_message()) + + def test_fails_for_extra_types(self): + issue = FakeIssue() + + # pylint: disable=attribute-defined-outside-init + issue.type = "some_new_type" + + api = self.api() + with patch.object(api, "issues", return_value=[issue]): + with self.assertRaises(ValueError): + api.release_message() + + def test_contains_issue_links(self): + api = self.api() + release_message = api.release_message() + for issue in api.issues(): + text = "[{number}]({url}) - {title}".format( + number=issue.number, url=issue.url, title=issue.title + ) + self.assertIn(text, release_message) + + +class TestGitHubAPICreateRelease(GitHubAPICaseWithIssues): """ Tests for :meth:`pywincffi.dev.release.GitHubAPI.create_release` """ - def setUp(self): - super(TestGitHubAPICreateRelease, self).setUp() - self.api = GitHubAPI(self.version) - mock = patch.object(self.api, "release_message", return_value="foobar") - mock.start() - self.addCleanup(mock.stop) - - def set_releases(self, value): - mock = patch.object(self.api.repo, "get_releases", return_value=value) - mock.start() - self.addCleanup(mock.stop) - return mock - def test_dry_run(self): - self.set_releases([]) - - # Exceptions will be raised if dry_run actually tries to do - # something - self.assertEqual(self.api.create_release(dry_run=True), "foobar") + api = self.api() + api.create_release(dry_run=True) + self.assertEqual(api.milestone.edit.call_count, 0) def test_closes_milestone(self): - self.set_releases([]) - - with patch.object(self.api.milestone, "edit") as mocked: - self.api.create_release(close_milestone=True) - - mocked.assert_called_with(self.version, state="closed") + api = self.api(releases=[]) + api.create_release(close_milestone=True) + api.milestone.edit.assert_called_with(api.version, state="closed") def test_create_tag_and_release_fails_without_recreate(self): - self.set_releases([Mock(tag_name=self.version)]) - + api = self.api() with self.assertRaisesRegex(RuntimeError, ".*%r already exists.*" % self.version): - self.api.create_release() + api.create_release() def test_create_tag_and_release_deletes_existing(self): - release_tag = Mock(tag_name=self.version) - self.set_releases([release_tag]) - self.api.create_release(recreate=True) - self.assertEqual(release_tag.delete_release.call_count, 1) + api = self.api() + api.create_release(recreate=True) - def test_create_tag_and_release_arguments(self): - self.set_releases([]) + # pylint: disable=no-member + get_releases = api.repo.get_releases.return_value[0] + get_releases.delete_release.assert_called_with() - with patch.object(self.api.repo, - "create_git_tag_and_release") as mocked: - self.api.create_release(recreate=True) + def test_create_tag_and_release_arguments(self): + api = self.api() + api.create_release(recreate=True) - mocked.assert_called_with( - self.api.version, + # pylint: disable=no-member + api.repo.create_git_tag_and_release.assert_called_with( + api.version, "Tagged by release.py", - self.version, - self.api.release_message(), - self.api.commit(), + api.version, + api.release_message(), + api.commit(), "commit", draft=True, prerelease=False ) diff --git a/tools/release.py b/tools/release.py index 804004a..ce08071 100755 --- a/tools/release.py +++ b/tools/release.py @@ -105,7 +105,7 @@ def main(): if github.milestone.state != "closed": should_continue( - "GitHub milestone %s is still open, continue? [y/n]" % version, + "GitHub milestone %s is still open, continue? [y/n] " % version, skip=args.confirm) release = github.create_release(