From e4190ff2790b836abaaebb2584cc57457725c6cf Mon Sep 17 00:00:00 2001 From: Darragh Bailey Date: Wed, 7 Dec 2016 15:01:47 +0000 Subject: [PATCH] Fix test and resulting random failures Ensure the test for checking "change merge result is identical from both upstream with carried patches and current" can be handled correctly is building the test tree correctly using 'cherry-pick' to duplicate the tree state. For cherry-pick of changes to work correct, enable use of 'force cherry-pick' through the '-x' option, to ensure a new commit is always made instead of allowing for the possibility of a fast-forward to occur. Include a modification to the commit message to insert the new node name in the message (e.g "[C] Add file change" is cherry-picked as "[C1] Add file change") This changes exposed a random bug where the walking of the graph results in the same node being visited twice. This in turn causes the wrong commit id to be stored and reused to checkout the desired node when building the tree resulting in a very broken graph. Add a check for the _FINISHED flag being set to spot this scenario and use this to avoid returning the same node twice in the topological sorted graph, which in turn ensures that we do not accidentally recreate a second commit for the same node, replacing the original node. Change-Id: Ie1ae27196b3616979826e2df72df3cb7a517475d --- git_upstream/tests/base.py | 4 +++- ..._merge_result_identical_on_both_sides.yaml | 20 ++++++++----------- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/git_upstream/tests/base.py b/git_upstream/tests/base.py index 377fde1..ba07704 100644 --- a/git_upstream/tests/base.py +++ b/git_upstream/tests/base.py @@ -70,6 +70,8 @@ def reverse_toposort(data): yield (node, data[node]) visited[node] = _FINISHED continue + elif visited.get(node) is _FINISHED: + continue visited[node] = _VISITED nodes_to_visit.append(node) @@ -205,7 +207,7 @@ class BuildTree(object): def _commit(self, node): p_node = _get_node_to_pick(node) if p_node: - self.git.cherry_pick(self.graph[p_node]) + self.git.cherry_pick(self.graph[p_node], x=True) else: # standard commit self.gitrepo.add_commits(1, ref="HEAD", diff --git a/git_upstream/tests/searchers/scenarios/changes_merge_result_identical_on_both_sides.yaml b/git_upstream/tests/searchers/scenarios/changes_merge_result_identical_on_both_sides.yaml index 59a917d..3c4cb1d 100644 --- a/git_upstream/tests/searchers/scenarios/changes_merge_result_identical_on_both_sides.yaml +++ b/git_upstream/tests/searchers/scenarios/changes_merge_result_identical_on_both_sides.yaml @@ -24,34 +24,30 @@ Repository layout being tested - B---C---D---G---H---I---J master - / / - / G1 import/next + B---C---D---G---H---I---J master / / - / B1--C1--D1 + / D1--G1--- / / - A---E---F---K---L upstream/master + A--B1--C1---K---L upstream/master tree: - [A, []] - [B, [A]] - [C, [B]] - [D, [C]] - - [E, [A]] - - [F, [E]] - - [G, [D]] - - [B1, [F]] + - [B1, [A]] - [C1, [B1]] + - [G, [D]] - [D1, [C1]] - [G1, [D1]] - - [H, [=G, G1]] + - [H, [G, =G1]] - [I, [H]] - [J, [I]] - - [K, [F]] + - [K, [C1]] - [L, [K]] branches: head: [master, J] upstream: [upstream/master, L] - expected-changes: [B1, C1, D1, G1, H, I, J] + expected-changes: [D1, G1, H, I, J]