Merge ~nacc/git-ubuntu:importer-get_changelog_parent_commit into git-ubuntu:master

[Nish Aravamudan]

Make jenkins happy.

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:01d65e1ab9610af305c09d3cb01ff3c26984195b
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/404/
Executed test runs:
    FAILED: VM Setup

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/404/rebuild

review: Needs Fixing (continuous-integration)

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:01d65e1ab9610af305c09d3cb01ff3c26984195b
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/405/
Executed test runs:
    SUCCESS: VM Setup
    SUCCESS: Build
    SUCCESS: Unit Tests
    SUCCESS: Integration Tests
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/405/rebuild

review: Approve (continuous-integration)

Unmerged commits

01d65e1... by Nish Aravamudan on 2018-04-10

importer: abstract out get_changelog_parent_commit

This was duplicated code between the import_{un,}applied_spi and needs
to be abstracted out anyways for future changes.

Add some unit tests. While not complete in and of themselves, the code
will be further tested with follow-on unit tests for the importer
methods.

There should be no functional change.

diff --git a/gitubuntu/importer.py b/gitubuntu/importer.py
index 32b74c8..a8d7ec6 100644
--- a/gitubuntu/importer.py
+++ b/gitubuntu/importer.py
@@ -1260,17 +1260,82 @@ def create_import_tag(repo, commit_hash, version, namespace):
         return repo.get_import_tag(version, namespace)
 
 
-# imports a package based upon source package information
-def import_unapplied_spi(
+def get_changelog_parent_commit(
     repo,
-    spi,
     namespace,
-    skip_orig,
     parent_overrides,
+    import_tree_versions,
+    patches_applied,
+):
+    """Walk changelog backwards until we find an imported version,
+    skipping the current version
+
+    :param repo gitubuntu.git_repository.GitUbuntuRepository The
+        repository to use
+    :param parent_overrides dict See parse_parentfile
+    :param import_tree_versions list(str) List of versions to iterate.
+    :param patches_applied bool If true, look for patches-applied
+        imports
+
+    :rtype str
+    :returns Git commit hash of the nearest (chronologically) imported
+        changelog version
+    """
+    # skip current version
+    import_tree_versions = import_tree_versions[1:]
+
+    for version in import_tree_versions:
+        if patches_applied:
+            changelog_parent_tag = repo.get_applied_tag(
+                version,
+                namespace,
+            )
+        else:
+            changelog_parent_tag = repo.get_import_tag(
+                version,
+                namespace,
+            ) or repo.get_orphan_tag(
+                version,
+                namespace,
+            )
+
+        if not changelog_parent_tag:
+            continue
+
+        # sanity check that version from d/changelog of the
+        # tagged parent matches ours
+        changelog_parent_version, _ = repo.get_changelog_versions_from_treeish(
+            str(changelog_parent_tag.peel(pygit2.Tree).id)
+        )
+
+        # if the parent was imported via a parent override
+        # itself, assume it is valid without checking its
+        # tree's debian/changelog, as it wasn't used
+        if (version in parent_overrides or
+            changelog_parent_version == version
+        ):
+            changelog_parent_commit = str(
+                changelog_parent_tag.peel(pygit2.Commit).id
+            )
+
+            logging.debug("Changelog parent (tag) is %s",
+                repo.tag_to_pretty_name(changelog_parent_tag)
+            )
+            return changelog_parent_commit
+
+    return None
+
+
+# imports a package based upon source package information
+def import_unapplied_spi(
+     repo,
+     spi,
+     namespace,
+     skip_orig,
+     parent_overrides,
 ):
     """Imports a source package from Launchpad into the git
     repository
-
     Arguments:
     spi - a SourcePackageInformation instance
     """
@@ -1381,42 +1446,13 @@ def import_unapplied_spi(
                  unapplied_tip_version,
             )
 
-        # Walk changelog backwards until we find an imported or orphaned
-        # version, preferring imported versions
-        for version in import_tree_versions:
-            unapplied_changelog_parent_tag = repo.get_import_tag(
-                version,
-                namespace,
-            ) or repo.get_orphan_tag(
-                version,
-                namespace,
-            )
-            if unapplied_changelog_parent_tag is not None:
-                # sanity check that version from d/changelog of the
-                # tagged parent matches ours
-                parent_changelog_version, _ = \
-                    repo.get_changelog_versions_from_treeish(
-                        str(unapplied_changelog_parent_tag.peel(pygit2.Tree).id),
-                                                            )
-                # if the parent was imported via a parent override
-                # itself, assume it is valid without checking its
-                # tree's debian/changelog, as it wasn't used
-                if (version not in parent_overrides and
-                    parent_changelog_version != version
-                ):
-                    logging.error(
-                        "Found a tag corresponding to parent version "
-                        "%s, but d/changelog version (%s) differs. Will "
-                        "orphan commit.",
-                        version,
-                        parent_changelog_version,
-                    )
-                else:
-                    unapplied_changelog_parent_commit = str(unapplied_changelog_parent_tag.peel().id)
-                    logging.debug("Changelog parent (tag) is %s",
-                        repo.tag_to_pretty_name(unapplied_changelog_parent_tag)
-                    )
-                break
+        unapplied_changelog_parent_commit = get_changelog_parent_commit(
+            repo,
+            namespace,
+            parent_overrides,
+            import_tree_versions,
+            patches_applied=False,
+        )
 
     # check if the version to import has already been uploaded to
     # this head -- we leverage the above code to perform a 'dry-run'
@@ -1572,36 +1608,14 @@ def import_applied_spi(
                          spi.version, pretty_head_name, applied_tip_version
                         )
 
-        # Walk changelog backwards until we find an imported version
-        for version in import_tree_versions:
-            applied_changelog_parent_tag = repo.get_applied_tag(version, namespace)
-            if applied_changelog_parent_tag is not None:
-                # sanity check that version from d/changelog of the
-                # tagged parent matches ours
-                parent_changelog_version, _ = \
-                    repo.get_changelog_versions_from_treeish(
-                        str(applied_changelog_parent_tag.peel(pygit2.Tree).id),
-                                                            )
-                # if the parent was imported via a parent override
-                # itself, assume it is valid without checking its
-                # tree's debian/changelog, as it wasn't used
-                if (version not in parent_overrides and
-                    parent_changelog_version != version
-                   ):
-                    logging.error('Found a tag corresponding to '
-                                  'parent version %s, but d/changelog '
-                                  'version (%s) differs. Will '
-                                  'orphan commit.' % (
-                                  version,
-                                  parent_changelog_version
-                                  )
-                                 )
-                else:
-                    applied_changelog_parent_commit = str(applied_changelog_parent_tag.peel().id)
-                    logging.debug('Changelog parent (tag) is %s',
-                        repo.tag_to_pretty_name(applied_changelog_parent_tag)
-                                )
-                break
+
+        applied_changelog_parent_commit = get_changelog_parent_commit(
+            repo,
+            namespace,
+            parent_overrides,
+            import_tree_versions,
+            patches_applied=True,
+        )
 
     applied_tag = repo.get_applied_tag(spi.version, namespace)
     if applied_tag:
diff --git a/gitubuntu/test_importer.py b/gitubuntu/test_importer.py
index 87db425..b7f24a1 100644
--- a/gitubuntu/test_importer.py
+++ b/gitubuntu/test_importer.py
@@ -482,3 +482,154 @@ def test_create_applied_tag(repo, input_data, expected_changes, test_refs):
         repoB=expected_result,
         test_refs=test_refs,
     )
+
+
[e-mail]('input_data, parent_overrides, changelog_versions, patches_applied, expected_ref',
+    [
+        (
+            repo_builder.Repo(
+                commit_list=[],
+                branches={},
+                tags={},
+            ),
+            {},
+            ['1-2', '1-1',],
+            False,
+            None,
+        ),
+        (
+            repo_builder.Repo(
+                commit_list=[
+                    repo_builder.Commit(
+                        tree=repo_builder.SourceTree(
+                            source_builder.Source(
+                                source_builder.SourceSpec(
+                                    version='1-1',
+                                    native=False,
+                                )
+                            )
+                        ),
+                        name='import',
+                    ),
+                ],
+                branches={
+                },
+                tags={
+                    'importer/import/1-1': repo_builder.Placeholder('import'),
+                },
+            ),
+            {},
+            ['1-2', '1-1'],
+            False,
+            'refs/tags/importer/import/1-1',
+        ),
+        (
+            repo_builder.Repo(
+                commit_list=[
+                    repo_builder.Commit(
+                        tree=repo_builder.SourceTree(
+                            source_builder.Source(
+                                source_builder.SourceSpec(
+                                    version='1-1',
+                                    native=False,
+                                )
+                            )
+                        ),
+                        name='import',
+                    ),
+                ],
+                branches={
+                },
+                tags={
+                    'importer/import/1-1': repo_builder.Placeholder('import'),
+                },
+            ),
+            {},
+            ['1-3', '1-2', '1-1'],
+            False,
+            'refs/tags/importer/import/1-1',
+        ),
+        (
+            repo_builder.Repo(
+                commit_list=[],
+                branches={},
+                tags={},
+            ),
+            {},
+            ['1-2', '1-1',],
+            True,
+            None,
+        ),
+        (
+            repo_builder.Repo(
+                commit_list=[
+                    repo_builder.Commit(
+                        tree=repo_builder.SourceTree(
+                            source_builder.Source(
+                                source_builder.SourceSpec(
+                                    version='1-1',
+                                    native=False,
+                                )
+                            )
+                        ),
+                        name='applied',
+                    ),
+                ],
+                branches={
+                },
+                tags={
+                    'importer/applied/1-1': repo_builder.Placeholder('applied'),
+                },
+            ),
+            {},
+            ['1-2', '1-1'],
+            True,
+            'refs/tags/importer/applied/1-1',
+        ),
+        (
+            repo_builder.Repo(
+                commit_list=[
+                    repo_builder.Commit(
+                        tree=repo_builder.SourceTree(
+                            source_builder.Source(
+                                source_builder.SourceSpec(
+                                    version='1-1',
+                                    native=False,
+                                )
+                            )
+                        ),
+                        name='applied',
+                    ),
+                ],
+                branches={
+                },
+                tags={
+                    'importer/applied/1-1': repo_builder.Placeholder('applied'),
+                },
+            ),
+            {},
+            ['1-3', '1-2', '1-1'],
+            True,
+            'refs/tags/importer/applied/1-1',
+        ),
+    ],
+)
+def test_get_changelog_parent_commit(
+    repo,
+    input_data,
+    parent_overrides,
+    changelog_versions,
+    patches_applied,
+    expected_ref,
+):
+    input_data.write(repo.raw_repo)
+    assert target.get_changelog_parent_commit(
+        repo,
+        'importer',
+        parent_overrides,
+        changelog_versions,
+        patches_applied,
+    ) == (
+        str(repo.raw_repo.lookup_reference(expected_ref).peel(pygit2.Commit).id)
+        if expected_ref else expected_ref
+    )