From a4062de4aeb2b12430a27286a9051b4b9338410e Mon Sep 17 00:00:00 2001 From: Darragh Bailey Date: Sat, 10 Mar 2018 11:17:45 +0000 Subject: [PATCH] Switch to using fixtures-git for testing Use library to handle setup of fixtures for testing with git. Change-Id: I5b39f8d52beca848adb20d646c26d91c4b977b92 --- git_upstream/tests/base.py | 318 ++---------------- git_upstream/tests/build-test-tree.py | 4 +- .../tests/searchers/test_searchers.py | 2 +- .../tests/strategies/test_strategies.py | 2 +- git_upstream/tests/test_commands.py | 3 +- git_upstream/tests/test_import.py | 9 +- test-requirements.txt | 2 +- 7 files changed, 36 insertions(+), 304 deletions(-) diff --git a/git_upstream/tests/base.py b/git_upstream/tests/base.py index 3f428bd..240f6bb 100644 --- a/git_upstream/tests/base.py +++ b/git_upstream/tests/base.py @@ -17,83 +17,15 @@ import io import logging import os from pprint import pformat -import re import subprocess -import tempfile -import faker import fixtures -import git +import fixtures_git import testtools from testtools.content import text_content import yaml -def _get_node_to_pick(node): - m = re.search(r'(.*)(\d+)$', node) - if m: - # get copy of a another change - node_number = int(m.group(2)) - 1 - node_name = m.group(1) - if node_number > 0: - node_name += str(node_number) - return node_name - return None - - -_NOT_VISITED = 0 -_VISITED = 1 -_FINISHED = 2 - - -def reverse_toposort(data): - - # convert to dict for linear lookup times when returning - data = dict(data) - - # keep track of nodes visited and processed - # by checking if a child has been visited before but not processed you - # can detect a back edge and abort since the graph is not acyclic - visited = dict() - - # DFS algorithm with customization to handle use of '=' notation for merge - # commits and also the additional dependency for cherry-picking - nodes_to_visit = [] - for i in data.keys(): - if i not in visited: - nodes_to_visit.append(i) - - while nodes_to_visit: - node = nodes_to_visit.pop() - if visited.get(node) is _VISITED: - # already visited so just return it with it's deps - yield (node, data[node]) - visited[node] = _FINISHED - continue - elif visited.get(node) is _FINISHED: - continue - - visited[node] = _VISITED - nodes_to_visit.append(node) - # special case for cherry-picking changes - c_node = _get_node_to_pick(node) - if c_node and c_node not in visited: - nodes_to_visit.append(c_node) - - for d in data[node]: - r_d = d.strip('=') - if r_d not in visited: - nodes_to_visit.append(r_d) - else: - # if we've already visited a dep but not processed it, - # then we have a back edge of some kind - if visited[r_d] is _VISITED: - message = ("Graph is not acyclic: %s is a dependency " - "of %s, but has been visited without being " - "processed before it." % (r_d, node)) - raise RuntimeError(message) - - def get_scenarios(scenarios_path): """Returns a list of scenarios, each scenario being describe by it's name, and a dict containing the parameters @@ -131,218 +63,6 @@ class DiveDir(fixtures.Fixture): os.chdir(self.path) -class GitRepo(fixtures.Fixture): - """Create an empty git repo in which to operate.""" - - def __init__(self, path=None): - self.path = path - self._faker = faker.Faker() - - def _setUp(self): - self._file_list = set() - if not self.path: - tempdir = self.useFixture(fixtures.TempDir()) - self.path = os.path.join(tempdir.path, 'git') - - os.mkdir(self.path) - g = git.Git(self.path) - g.init() - - self.repo = git.Repo(self.path) - self.repo.git.config('user.email', 'user@example.com') - self.repo.git.config('user.name', 'Example User') - self._create_file_commit() - - def _create_file(self, contents=None): - if not contents: - contents = "\n\n".join(self._faker.paragraphs(3)) - - # always want to ensure the files added to the repo are unique no - # matter which branch they are added to, as otherwise there may - # be conflicts caused by replaying local changes and performing - # merges - while True: - tmpfile = tempfile.NamedTemporaryFile(dir=self.repo.working_dir, - delete=False) - if tmpfile.name not in self._file_list: - self._file_list.add(tmpfile.name) - break - tmpfile.close() - os.remove(tmpfile.name) - tmpfile.write(contents.encode('utf-8')) - tmpfile.close() - return tmpfile.name - - def _create_file_commit(self, change_id=None, message_prefix=None): - filename = self._create_file() - self.repo.git.add(filename) - message = "Adding %s" % os.path.basename(filename) - if message_prefix: - message = "%s %s" % (message_prefix, message) - if change_id: - message = message + "\n\nChange-Id: %s" % change_id - self.repo.git.commit(m=message) - - def add_commits(self, num=1, ref="HEAD", change_ids=[], - message_prefix=None): - """Create the given number of commits using generated files""" - if ref != "HEAD": - self.repo.git.checkout(ref) - - num = max(num, len(change_ids)) - ids = list(change_ids) + [None] * (num - len(change_ids)) - - for x in range(num): - self._create_file_commit(ids[x], message_prefix=message_prefix) - - -class BuildTree(object): - - def __init__(self, gitrepo, tree, branches): - self.graph = {} - self.gitrepo = gitrepo - self.repo = gitrepo.repo - self.git = self.gitrepo.repo.git - self._build_git_tree(tree, branches) - - def _commit(self, node): - p_node = _get_node_to_pick(node) - if p_node: - self.git.cherry_pick(self.graph[p_node], x=True) - else: - # standard commit - self.gitrepo.add_commits(1, ref="HEAD", - message_prefix="[%s]" % node) - - def _merge_commit(self, node, parents): - # merge commits - parent_nodes = [p.lstrip("=") for p in parents] - commits = [str(self.graph[p]) for p in parent_nodes[1:]] - - if any([p.startswith("=") for p in parents]): - # special merge commit using inverse of 'ours' by - # emptying the current index and then reading in any - # trees of the nodes prefixed with '=' - use = [str(self.graph[p.lstrip("=")]) - for p in parents if p.startswith("=")] - try: - self.git.merge(*commits, s="ours", no_commit=True) - except git.exc.GitCommandError as exc: - if 'refusing to merge unrelated histories' in exc.stderr: - self.git.merge(*commits, s="ours", no_commit=True, - allow_unrelated_histories=True) - else: - raise - self.git.read_tree(empty=True) - self.git.read_tree(*use, u=True, reset=True) - elif len(commits) < 2: - # standard merge - try: - self.git.merge(*commits, no_commit=True) - except git.exc.GitCommandError as exc: - if 'refusing to merge unrelated histories' in exc.stderr: - self.git.merge(*commits, no_commit=True, - allow_unrelated_histories=True) - else: - raise - else: - # multi-branch merge, git is not great at handling - # merging multiple orphaned branches - try: - self.git.merge(*commits, s="ours", no_commit=True) - except git.exc.GitCommandError as exc: - if 'refusing to merge unrelated histories' in exc.stderr: - self.git.merge(*commits, s="ours", no_commit=True, - allow_unrelated_histories=True) - else: - raise - self.git.read_tree(empty=True) - self.git.read_tree("HEAD", *commits) - self.git.checkout("--", ".") - self.git.commit(m="[%s] Merging %s into %s" % - (node, ",".join(parent_nodes[1:]), - parent_nodes[0])) - self.git.clean(f=True, d=True, x=True) - - def _build_git_tree(self, graph_def, branches=[]): - """Helper function to build a git repository from a graph definition - of nodes and their parent nodes. A list of branches may be provided - where each element has two members corresponding to the name and the - target node it references. - - Supports unordered graphs, only requirement is that there is a commit - defined with no parents, which will become the root commit. - - Root commits can specified by an empty list as the second member: - - ('NodeA', []) - - Merge commits are specified by multiple nodes: - - ('NodeMerge', ['Node1', 'Node2']) - - - As the import subcommand to git-upstream supports a special merge - commit that ignores all previous history from the other tree being - merged in using the 'ours' strategy. You specify this by defining - a parent node as '='. The resulting merge commit contains just - the contents of the tree from the specified parent while still - recording the parents. - - Following will result in a merge commit 'C', with parents 'P1' and - 'P2', but will have the same tree as 'P1'. - - ('C', ['=P1', 'P2']) - - - The tree building code can handle a graph definition being out of - order but will fail to find certain circular dependencies and may - result in an infinite loop. - - Examples: - - [('A', []), ('B', ['A']), ('C', ['B'])] - [('A', []), ('C', ['B']), ('B', ['A'])] - """ - - # require that graphs must have at least 1 node with no - # parents, which is a root commit in git - if not any([True for _, parents in graph_def if not parents]): - assert("No root commit defined in test graph") - - for node, parents in reverse_toposort(graph_def): - if not parents: - # root commit - self.git.symbolic_ref("HEAD", "refs/heads/%s" % node) - self.git.rm(".", r=True, cached=True) - self.git.clean(f=True, d=True, x=True) - self.gitrepo.add_commits(1, ref="HEAD", - message_prefix="[%s]" % node) - # only explicitly listed branches should exist afterwards - self.git.checkout(self.repo.commit()) - self.git.branch(node, D=True) - - else: - # checkout the dependent node - self.git.checkout(self.graph[parents[0].lstrip('=')]) - if len(parents) > 1: - # merge commits - self._merge_commit(node, parents) - else: - self._commit(node) - self.graph[node] = self.repo.commit() - - for name, node in branches: - self.git.branch(name, str(self.graph[node]), f=True) - - # return to master - self.git.checkout("master") - - def _commits_from_nodes(self, nodes=[]): - - return [self.graph[n] for n in nodes] - - class BaseTestCase(testtools.TestCase): """Base Test Case for all tests.""" @@ -352,14 +72,6 @@ class BaseTestCase(testtools.TestCase): super(BaseTestCase, self).setUp() self.logger = self.useFixture(fixtures.FakeLogger(level=logging.DEBUG)) - self.testrepo = self.useFixture(GitRepo()) - repo_path = self.testrepo.path - self.useFixture(DiveDir(repo_path)) - - self.repo = self.testrepo.repo - self.git = self.repo.git - - self.addOnException(self.attach_graph_info) # _testMethodDoc is a hidden attribute containing the docstring for # the given test, useful for some tests where description is not @@ -367,10 +79,25 @@ class BaseTestCase(testtools.TestCase): if getattr(self, '_testMethodDoc', None): self.addDetail('description', text_content(self._testMethodDoc)) - self.gittree = None if hasattr(self, 'tree'): - self.gittree = BuildTree( - self.testrepo, self.tree, self.branches.values()) + self.testrepo = self.useFixture( + fixtures_git.GitFixture( + graph=self.tree, + branches=self.branches.values(), + ) + ) + else: + self.testrepo = self.useFixture( + fixtures_git.GitFixture()) + + self.gittree = self.testrepo.gittree + + self.useFixture(DiveDir(self.testrepo.path)) + + self.repo = self.testrepo.repo + self.git = self.repo.git + + self.addOnException(self.attach_repo_info) if hasattr(self, 'pre_script'): self.run_pre_script() @@ -426,16 +153,17 @@ class BaseTestCase(testtools.TestCase): self.addDetail('post-script-output', text_content(output.decode('utf-8'))) - def attach_graph_info(self, exc_info): + def attach_repo_info(self, exc_info): # appears to be an odd bug where this method is called twice # if registered with addOnException so make sure to guard # that the path actually exists before running - if not (hasattr(self.testrepo, 'path') and - os.path.exists(self.testrepo.path)): + if not os.path.exists(self.testrepo.path): return + # in case we haven't setup yet if not self.gittree or not self.gittree.graph: return + self.addDetail('graph-dict', text_content(pformat(self.gittree.graph))) self.addDetail( diff --git a/git_upstream/tests/build-test-tree.py b/git_upstream/tests/build-test-tree.py index 5b11c79..c9828ab 100644 --- a/git_upstream/tests/build-test-tree.py +++ b/git_upstream/tests/build-test-tree.py @@ -42,8 +42,8 @@ def build_test_tree(basedir, filename): with base.DiveDir(testrepo.path): if 'tree' in data: - base.BuildTree(testrepo, data['tree'], - data['branches'].values()) + base.fixtures_git.gitfixture.GitTree( + testrepo, data['tree'], data['branches'].values()) if 'pre-script' in data: output = subprocess.check_output(data['pre-script'], diff --git a/git_upstream/tests/searchers/test_searchers.py b/git_upstream/tests/searchers/test_searchers.py index d25dd04..f18f64b 100644 --- a/git_upstream/tests/searchers/test_searchers.py +++ b/git_upstream/tests/searchers/test_searchers.py @@ -53,7 +53,7 @@ class TestUpstreamMergeBaseSearcher(TestWithScenarios, BaseTestCase): patterns=pattern, repo=self.repo) self.assertEqual( - self.gittree._commits_from_nodes(reversed(self.expected_changes)), + self.gittree.commits_from_nodes(reversed(self.expected_changes)), searcher.list(self.branches['upstream'][0])) self.assertThat(self.logger.output, diff --git a/git_upstream/tests/strategies/test_strategies.py b/git_upstream/tests/strategies/test_strategies.py index e88426a..59d0dd8 100644 --- a/git_upstream/tests/strategies/test_strategies.py +++ b/git_upstream/tests/strategies/test_strategies.py @@ -55,7 +55,7 @@ class TestStrategies(TestWithScenarios, BaseTestCase): search_refs=[self.branches['upstream'][0]]) self.assertEqual( - self.gittree._commits_from_nodes(self.expected_changes), + self.gittree.commits_from_nodes(self.expected_changes), [c for c in strategy.filtered_iter()]) self.assertThat(self.logger.output, diff --git a/git_upstream/tests/test_commands.py b/git_upstream/tests/test_commands.py index ccd0f89..d6531d8 100644 --- a/git_upstream/tests/test_commands.py +++ b/git_upstream/tests/test_commands.py @@ -94,7 +94,8 @@ class TestLoggingOptions(base.BaseTestCase): 'upstream': ('upstream/master', 'E'), } - self.gittree = base.BuildTree(self.testrepo, tree, branches.values()) + self.gittree = base.fixtures_git.gitfixture.GitTree( + self.testrepo, tree, branches.values()) cmdline = self.parser.get_default('script_cmdline') diff --git a/git_upstream/tests/test_import.py b/git_upstream/tests/test_import.py index 5777ee1..0e75100 100644 --- a/git_upstream/tests/test_import.py +++ b/git_upstream/tests/test_import.py @@ -57,7 +57,8 @@ class TestImport(base.BaseTestCase): 'import': ('import', 'C1') } - self.gittree = base.BuildTree(self.testrepo, tree, branches.values()) + self.gittree = base.fixtures_git.gitfixture.GitTree( + self.testrepo, tree, branches.values()) iu = ImportUpstream("master", "upstream/master", "import") iu.finish() self.assertEqual("", self.git.status(porcelain=True), @@ -94,7 +95,8 @@ class TestImport(base.BaseTestCase): 'import': ('import', 'C1') } - self.gittree = base.BuildTree(self.testrepo, tree, branches.values()) + self.gittree = base.fixtures_git.gitfixture.GitTree( + self.testrepo, tree, branches.values()) iu = ImportUpstream("master", "upstream/master", "import") # create a dummy file open('dummy-file', 'a').close() @@ -127,7 +129,8 @@ class TestImport(base.BaseTestCase): 'upstream': ('upstream/master', 'E'), } - self.gittree = base.BuildTree(self.testrepo, tree, branches.values()) + self.gittree = base.fixtures_git.gitfixture.GitTree( + self.testrepo, tree, branches.values()) self.git.tag("tag-um-1", "upstream/master") self.git.tag("tag-um-2", "upstream/master") iu = ImportUpstream("master", "tag-um-1", "import/{describe}") diff --git a/test-requirements.txt b/test-requirements.txt index 9317ecb..915b965 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,5 +1,5 @@ hacking>=0.11.0,<0.12 # Apache-2.0 -faker>=0.8.16 +fixtures-git>=0.1.0 # Apache-2.0 mock>=2.0.0 sphinx>=1.5.1,!=1.6.6,!=1.6.7,<2.0.0;python_version=='2.7' # BSD sphinx>=1.5.1,!=1.6.6,!=1.6.7,!=2.1.0;python_version>='3.4' # BSD