Merge lp:~dpb/latch-test/archive-artifacts-1493373 into lp:latch-test

[David Britton]

Respect 'artifacts' and 'artifacts_dest' tarmac config settings for latch.

Respect 'artifacts' and 'artifacts_dest' tarmac config settings for latch.

[🤖 Landscape Builder (landscape-builder)]

Command: make test
Result: Success
Revno: 19
Branch: lp:~davidpbritton/latch-test/archive-artifacts-1493373
Jenkins: https://ci.lscape.net/job/latch-test/2434/

review: Approve (test results)

[🤖 Landscape Builder (landscape-builder)]

Command: make test
Result: Success
Revno: 20
Branch: lp:~davidpbritton/latch-test/archive-artifacts-1493373
Jenkins: https://ci.lscape.net/job/latch-test/2435/

review: Approve (test results)

[Adam Collard (adam-collard)]

Minor nit picks inline.

review: Approve

[Alberto Donato (ack)]

+1. Looks good, a few minor comments inline.

[Alberto Donato (ack)]

review: Approve

=== modified file 'README'
--- README	2015-02-23 16:38:44 +0000
+++ README	2015-10-01 16:17:18 +0000
@@ -41,6 +41,10 @@
  * [Sections] - correspond to target branches, scanned for landing_candidates
                 (merge proposals).
  * verify_command - command to run to test the branch.
+ * artifacts - space separated list of artifacts to save. Paths should be
+               relative to the branch root.
+ * artifacts_dest - directory name in CWD where artifacts will be copied to.
+                    if unspecified, `latch-artifacts` is used.
 
 Everything outside this will be ignored for now.
 
@@ -59,8 +63,6 @@
  builds on other projects' functionality in jenkins, combined with the
  'parameters' file of that feature.
 
-In the future, more integration will be added.
-
 
 Exported Environment
 --------------------

=== modified file 'config'
--- config	2015-02-18 00:05:22 +0000
+++ config	2015-10-01 16:17:18 +0000
@@ -4,3 +4,4 @@
 verify_command = make test
 
 [lp:latch-test]
+artifacts = config

=== added directory 'fixtures'
=== added directory 'fixtures/artifacts'
=== added directory 'fixtures/artifacts/dir1'
=== added file 'fixtures/artifacts/dir1/file1'
=== added file 'fixtures/artifacts/dir1/file2'
=== added directory 'fixtures/artifacts/dir2'
=== added file 'fixtures/artifacts/file1'
=== added file 'fixtures/artifacts/file2'
=== added file 'fixtures/config'
--- fixtures/config	1970-01-01 00:00:00 +0000
+++ fixtures/config	2015-10-01 16:17:18 +0000
@@ -0,0 +1,6 @@
+# Example configuration for latch pointed at lp:latch-test, expected that you will
+# customize this for your project.
+[DEFAULT]
+verify_command = make test
+
+[lp:latch-test]

=== modified file 'latch.py'
--- latch.py	2015-09-10 13:30:35 +0000
+++ latch.py	2015-10-01 16:17:18 +0000
@@ -95,12 +95,6 @@
             stream=sys.stderr, level=level,
             format='%(asctime)s %(levelname)s %(message)s')
 
-    def _parse_config(self):
-        """Parse the configuration, return config object."""
-        config = ConfigParser.ConfigParser()
-        config.read(join(dirname(abspath(__file__)), "config"))
-        return config
-
     def scan_branch(self, branch):
         """Scan all MPs for given branch, return candidates to be tested."""
         lp_candidates = []
@@ -139,15 +133,22 @@
         self._lp_login(credentials_file=args.credentials_file)
         return args
 
-    def _get_config(self):
-        config = ConfigParser.ConfigParser()
-        config.read(join(dirname(abspath(__file__)), "config"))
+    def _get_config(self, config_file=None):
+        """Parse the configuration, return config object."""
+        if config_file is None:
+            config_file = join(dirname(abspath(__file__)), "config")
+        config = ConfigParser.ConfigParser({
+            "artifacts_dest": "latch-artifacts",
+            "artifacts": ""})
+        config.read(config_file)
         return config
 
-    def _lookup_config(self, config, lp_candidate, option):
-        """Return a config in the context of the lp_candidate."""
+    def _lookup_config(self, lp_candidate, option, config=None):
+        """Return a config setting in the context of the lp_candidate."""
+        if config is None:
+            config = self._get_config()
         bzr_id = lp_candidate.target_branch.bzr_identity
-        if bzr_id in config.sections():
+        if config.has_section(bzr_id):
             try:
                 return config.get(bzr_id, option)
             except ConfigParser.NoOptionError:
@@ -176,16 +177,20 @@
             "candidate", metavar="CANDIDATE",
             help="Candidate api url link to test.")
         args = self._parse_args(parser)
-        config = self._get_config()
         lp_candidate = self.lp.load(args.candidate)
         try:
             verify_command = self._lookup_config(
-                config, lp_candidate, "verify_command")
+                lp_candidate, "verify_command")
+            artifacts = self._lookup_config(
+                lp_candidate, "artifacts").split()
+            artifacts_dest = self._lookup_config(
+                lp_candidate, "artifacts_dest")
         except MissingConfigurationError:
             sys.exit(1)
         candidate = Candidate(self.lp, lp_candidate, readonly=self.readonly)
         environment = TestEnvironment(
-            candidate, self.lp, verify_command)
+            candidate, self.lp, verify_command, artifacts=artifacts,
+            artifacts_dest=artifacts_dest)
         with temporary_directory() as temp_dir:
             environment.initialize(temp_dir)
             result = environment.execute_test()
@@ -306,7 +311,9 @@
 
 
 class TestEnvironment(object):
-    def __init__(self, candidate, lp, verify_command="make", environ=None):
+    def __init__(
+            self, candidate, lp, verify_command="make", environ=None,
+            artifacts=None, artifacts_dest=None):
         self.candidate = candidate
         self.lp_candidate = candidate.lp_candidate
         self.lp = lp
@@ -314,6 +321,11 @@
         self.environ = environ
         if environ is None:
             self.environ = os.environ
+        self.artifacts = artifacts if artifacts else []
+        if artifacts_dest is None:
+            self.artifacts_dest = os.path.join(os.getcwd(), "latch-artifacts")
+        else:
+            self.artifacts_dest = artifacts_dest
 
     def initialize(self, root_dir):
         """
@@ -378,8 +390,24 @@
             result["status"] = u"Success"
         return result
 
+    def _copy_artifacts(self):
+        """Copy artifacts from self.source_dir into self.artifacts_dest."""
+        for artifact in self.artifacts:
+            artifact_src = os.path.join(self.source_dir, artifact)
+            artifact_dst = os.path.join(self.artifacts_dest, artifact)
+            log.debug("Copy from %s to %s" % (artifact_src, artifact_dst))
+            if not os.path.exists(artifact_src):
+                log.warning("Missing artifact: %s" % artifact_src)
+            elif os.path.isdir(artifact_src):
+                shutil.copytree(artifact_src, artifact_dst)
+            else:
+                if not os.path.exists(os.path.dirname(artifact_dst)):
+                    os.makedirs(os.path.dirname(artifact_dst))
+                shutil.copy(artifact_src, artifact_dst)
+
     def execute_test(self):
         """Test the local copy of the code, report on results in the MP."""
         result = self._verify()
+        self._copy_artifacts()
         self.candidate.add_result(result)
         return result

=== modified file 'test_latch.py'
--- test_latch.py	2015-09-10 13:30:35 +0000
+++ test_latch.py	2015-10-01 16:17:18 +0000
@@ -1,13 +1,16 @@
 # -*- coding: utf-8 -*-
 import ConfigParser
 import latch
+import filecmp
 import unittest
 from mock import MagicMock, patch
 import logging
 from logging.handlers import BufferingHandler
 import os
 import random
+import shutil
 from subprocess import PIPE, STDOUT, check_call, CalledProcessError
+import tempfile
 
 OUTPUT = u"output ✈"
 MULTILINE_OUTPUT = u"multiline\noutput ✈"
@@ -204,7 +207,7 @@
         config = ConfigParser.ConfigParser({"key": "value"})
         config.add_section("target_branch_id")
         self.assertEqual(
-            self.latch._lookup_config(config, candidate, "key"),
+            self.latch._lookup_config(candidate, "key", config=config),
             "value")
 
     def test__lookup_config_no_section(self):
@@ -215,7 +218,7 @@
         with self.assertRaisesRegexp(
                 latch.MissingConfigurationError,
                 ".*target_branch_id not in my configuration.*"):
-            self.latch._lookup_config(config, candidate, "key")
+            self.latch._lookup_config(candidate, "key", config=config)
 
     def test__lookup_config_no_key(self):
         """section exists, key/value does not."""
@@ -225,7 +228,18 @@
         with self.assertRaisesRegexp(
                 latch.MissingConfigurationError,
                 ".*key not defined.*"):
-            self.latch._lookup_config(config, candidate, "missing_key")
+            self.latch._lookup_config(candidate, "missing_key", config=config)
+
+    def test__get_config_defaults_are_set(self):
+        """_get_config has default settings applied."""
+        config_file = os.path.join(
+            os.path.dirname(__file__), "fixtures/config")
+        config = self.latch._get_config(config_file=config_file)
+        candidate = "lp:latch-test"
+        self.assertEqual(config.get(candidate, "artifacts"), "")
+        self.assertEqual(
+            config.get(candidate, "artifacts_dest"),
+            "latch-artifacts")
 
 
 class CandidateTest(LoggedTest):
@@ -432,15 +446,23 @@
 
 class TestEnvironmentTest(unittest.TestCase):
     def setUp(self):
+        self.tempcwd = tempfile.mkdtemp()
+        self.oldcwd = os.getcwd()
+        os.chdir(self.tempcwd)
         self.lp = MagicMock()
         self.lp.me = FAKE_ME
         self.environment = latch.TestEnvironment(
             latch.Candidate(self.lp, get_candidate(self.lp.me.name)),
             self.lp,
             verify_command="fake",
-            environ={})
+            environ={},
+            artifacts=[".bzr"])
         self.environment.source_dir = "source_dir"
 
+    def tearDown(self):
+        os.chdir(self.oldcwd)
+        shutil.rmtree(self.tempcwd)
+
     def test_initialize(self):
         """
         Call the TestEnvironment initialize() method, verify that
@@ -516,3 +538,78 @@
                 vote=candidate.vote_lookup[result["status"]],
                 subject="Latch Results: %(branch)s r%(revno)s" % result,
                 review_type="test results")
+            self.assertIn("latch-artifacts", os.listdir(os.getcwd()))
+            self.assertEqual(
+                os.listdir(os.path.join(os.getcwd(), "latch-artifacts")),
+                [".bzr"])
+
+
+class TestArtifactCopy(unittest.TestCase):
+
+    def _get_fake_test_environment(self, artifacts=None, artifacts_dest=None):
+        """Return a fake test environment, ready to use."""
+        lp = MagicMock()
+        env = latch.TestEnvironment(
+            latch.Candidate(lp, get_candidate(lp.me.name)),
+            lp,
+            verify_command="fake",
+            environ={},
+            artifacts=artifacts,
+            artifacts_dest=artifacts_dest)
+        env.source_dir = os.path.join(
+            os.path.dirname(__file__), "fixtures/artifacts")
+        return env
+
+    def setUp(self):
+        # Change my CWD to a tempdir
+        self.tempdir = tempfile.mkdtemp()
+        self.oldcwd = os.getcwd()
+        os.chdir(self.tempdir)
+
+    def tearDown(self):
+        os.chdir(self.oldcwd)
+        shutil.rmtree(self.tempdir)
+
+    def test_run_empty_config(self):
+        """No artifacts requested, should not create anything."""
+        env = self._get_fake_test_environment()
+        env._copy_artifacts()
+        self.assertEqual(os.listdir(os.getcwd()), [])
+
+    def test_run_copy_file_change_dest(self):
+        """Copy a single sample file, change the dest directory name"""
+        env = self._get_fake_test_environment(["file2"], "foo-my-dest")
+        env._copy_artifacts()
+        self.assertEqual(os.listdir(os.getcwd()), ["foo-my-dest"])
+        self.assertEqual(
+            os.listdir(os.path.join(os.getcwd(), "foo-my-dest")),
+            ["file2"])
+
+    def test_run_copy_all(self):
+        """Copy all components to check files, directories, multiples.
+
+        Also use the default dest directory name."""
+        env = self._get_fake_test_environment([
+            "file1", "file2", "dir1", "dir2"])
+        env._copy_artifacts()
+        comparison = filecmp.dircmp(
+            env.source_dir,
+            os.path.join(os.getcwd(), "latch-artifacts"))
+        self.assertItemsEqual(comparison.common_files, ['file1', 'file2'])
+        self.assertItemsEqual(comparison.common_dirs, ['dir1', 'dir2'])
+        self.assertItemsEqual(comparison.diff_files, [])
+        self.assertItemsEqual(comparison.left_only, [])
+        self.assertItemsEqual(comparison.right_only, [])
+
+    def test_run_copy_missing_artifact(self):
+        """Copy a single sample file, that is not there.
+
+        No artifacts should be copied, warning logged."""
+        log_mock = MagicMock()
+        latch.log = log_mock
+        env = self._get_fake_test_environment(["foo-file-not-found"])
+        src_path = os.path.join(env.source_dir, "foo-file-not-found")
+        env._copy_artifacts()
+        self.assertEqual(os.listdir(os.getcwd()), [])
+        log_mock.warning.assert_called_once_with(
+            "Missing artifact: %s" % src_path)