From bd0d908b96904035ad5d744e99da4c49213244d0 Mon Sep 17 00:00:00 2001 From: Darragh Bailey Date: Fri, 7 Dec 2018 11:08:30 +0000 Subject: [PATCH] Fix temp name creation and cleanup Allow the system to delete the file immediately as once the name is generated there is no further need of the temp file. Fix the temp name splitting and generation to handle being provided a string without the 'XXXXXX' template as part of it. Temporarily disable the requirements.txt check/gate as github3.py is not part of global-requirements. Change-Id: I14e56b0d4248dc037fab102acf3cb30573bfba8d --- .zuul.yaml | 6 +++ fixtures_git/github.py | 25 ++++++------- test-requirements.txt | 2 + tests/unit/test_github.py | 77 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 97 insertions(+), 13 deletions(-) create mode 100644 tests/unit/test_github.py diff --git a/.zuul.yaml b/.zuul.yaml index 79e599b..c59a67f 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -8,3 +8,9 @@ jobs: - build-openstack-sphinx-docs: voting: false + - requirements-check: + voting: false + gate: + jobs: + - requirements-check: + voting: false diff --git a/fixtures_git/github.py b/fixtures_git/github.py index c0febb2..f1ddef3 100644 --- a/fixtures_git/github.py +++ b/fixtures_git/github.py @@ -12,9 +12,8 @@ # License for the specific language governing permissions and limitations # under the License. -import os -import tempfile import time +import uuid import fixtures import github3 as github @@ -35,9 +34,9 @@ class GithubLoginMixin(object): url = "https://github.com" if parse.urlparse(url).netloc == "github.com": - self.github = github.login(token=token) + return github.login(token=token) else: - self.github = github.enterprise_login(token=token, url=url) + return github.enterprise_login(token=token, url=url) class GithubRepoFixture(GithubLoginMixin, fixtures.Fixture): @@ -58,21 +57,21 @@ class GithubRepoFixture(GithubLoginMixin, fixtures.Fixture): self.repo_name = None # use GithubLoginMixin - self.login(token, url) + self.github = self.login(token, url) # try an auth'ed request to make sure we have a valid token # note this requires the token to have read on user self.me = self.github.me() def _setUp(self): - template_parts = self.name_template.split('XXXXXX') - prefix = template_parts[0] - suffix = template_parts[-1] - tfile = tempfile.NamedTemporaryFile( - suffix=suffix, prefix=prefix, delete=False) - self.addCleanup(os.remove, tfile.name) - self.repo_name = os.path.basename(tfile.name) + # handle template_name missing 'XXXXX' result in it containing + # a single element so set suffix to '' in that case. + template_parts = iter(self.name_template.split('XXXXXX')) + prefix = next(template_parts) + suffix = next(template_parts, '') + + self.repo_name = ''.join([prefix, str(uuid.uuid4())[:8], suffix]) self.addCleanup(self._delete_repo) @@ -108,7 +107,7 @@ class GithubForkedRepoFixture(GithubLoginMixin, fixtures.Fixture): self.repo = None # use GithubLoginMixin - self.login(token, url) + self.github = self.login(token, url) # try an auth'ed request to make sure we have a valid token # note this requires the token to have read on user diff --git a/test-requirements.txt b/test-requirements.txt index c0eca26..c9dbbe5 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,5 +1,7 @@ flake8 +github3.py hacking +mock sphinx>=1.6.5 stestr>=1.0.0 testtools>=2.2.0 diff --git a/tests/unit/test_github.py b/tests/unit/test_github.py new file mode 100644 index 0000000..b2f61e4 --- /dev/null +++ b/tests/unit/test_github.py @@ -0,0 +1,77 @@ +# Copyright (c) 2018 Hewlett Packard Enterprise Development LP +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +import mock +import testtools + +from fixtures_git import github as gh_fixture + + +class TestGithubRepoFixture(testtools.TestCase): + + @mock.patch('fixtures_git.github.GithubRepoFixture.login', + mock.Mock(return_value=mock.Mock())) + def test_tempname_default(self): + + gh_repo = gh_fixture.GithubRepoFixture('owner', 'token') + gh_repo.setUp() + self.assertThat( + gh_repo.repo_name, + testtools.matchers.StartsWith('workflow-test-') + ) + self.assertThat( + gh_repo.repo_name, + testtools.matchers.NotEquals('workflow-test-') + ) + + @mock.patch('fixtures_git.github.GithubRepoFixture.login', + mock.Mock(return_value=mock.Mock())) + def test_tempname_custom(self): + + template = 'my-custom-tmp-XXXXXX-template' + gh_repo = gh_fixture.GithubRepoFixture('owner', 'token', + name_template=template) + gh_repo.setUp() + self.assertThat( + gh_repo.repo_name, + testtools.matchers.StartsWith(template.split('XXXXXX')[0]) + ) + self.assertThat( + gh_repo.repo_name, + testtools.matchers.EndsWith(template.split('XXXXXX')[1]) + ) + + @mock.patch('fixtures_git.github.GithubRepoFixture.login', + mock.Mock(return_value=mock.Mock())) + def test_tempname_no_suffix_in_template(self): + + template = 'my-custom-tmp-' + gh_repo = gh_fixture.GithubRepoFixture('owner', 'token', + name_template=template) + gh_repo.setUp() + self.assertThat( + gh_repo.repo_name, + testtools.matchers.StartsWith(template) + ) + self.assertThat( + gh_repo.repo_name, + testtools.matchers.Not( + testtools.matchers.Equals(template.split('XXXXXX')[-1])) + ) + self.assertThat( + gh_repo.repo_name.split('-')[-1], + testtools.matchers.MatchesRegex('[a-f0-9]{8}') + )