Merge lp:~jtv/maas-test/copy-file into lp:maas-test

[Jeroen T. Vermeulen]

New KVMFixture method: upload_file, needed for uploading bootresources.yaml. Also, move make_file() test helper into a new factory module.

This is preparation for fixing bug 1302608.

Jeroen

[Julian Edwards (julian-edwards)]

This looks basically OK.

I am a little confused on how Python lets you have two make_file functions in test_maasfixture.py. Why don't you just import the one you need?

review: Approve

[Jeroen T. Vermeulen (jtv)]

It was just a way to keep the calls to the pre-existing TestMAASFixture.make_file method unchanged, while still introducing the global make_file function elsewhere. Removed now.

[MAAS Lander (maas-lander)]

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

=== modified file 'maastest/kvmfixture.py'
--- maastest/kvmfixture.py	2014-02-26 21:05:58 +0000
+++ maastest/kvmfixture.py	2014-04-09 06:59:27 +0000
@@ -1,4 +1,4 @@
-# Copyright 2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2013-2014 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Manage a KVM machine."""
@@ -346,6 +346,19 @@
         self.running = False
         logging.info("Done destroying virtual machine %s.", self.name)
 
+    def _get_base_ssh_options(self):
+        """Return a list of ssh options for connecting to the VM.
+
+        These are minimal options that are needed to connect at all, whether
+        using `ssh` or `scp`.
+        """
+        return [
+            "-i", self.ssh_private_key_file,
+            "-o", "LogLevel=quiet",
+            "-o", "UserKnownHostsFile=/dev/null",
+            "-o", "StrictHostKeyChecking=no",
+            ]
+
     def _make_ssh_command(self, command_line):
         """Compose an `ssh` command line to execute `command_line` in the VM.
 
@@ -358,10 +371,7 @@
         remote_command_line = ' '.join(map(quote, command_line))
         return [
             "ssh",
-            "-i", self.ssh_private_key_file,
-            "-o", "LogLevel=quiet",
-            "-o", "UserKnownHostsFile=/dev/null",
-            "-o", "StrictHostKeyChecking=no",
+            ] + self._get_base_ssh_options() + [
             self.identity(),
             "LC_ALL=%s" % VM_LOCALE,
             remote_command_line,
@@ -405,6 +415,20 @@
         self.addDetail(cmd_prefix + ' stderr', text_content(u_stderr))
         return retcode, stdout, stderr
 
+    def upload_file(self, source, dest=None):
+        """Copy the given file into the virtual machine's filesystem.
+
+        :param source: File to copy.
+        :param dest: Optional destination.  May be an absolute path, or one
+            relative to the `ubuntu` user's home directory.  Defaults to the
+            same base name as `source`, directly in the user's home directory.
+        """
+        if dest is None:
+            dest = os.path.basename(source)
+        run_command(
+            ['scp'] + self._get_base_ssh_options() + [source, dest],
+            check_call=True)
+
     def get_ip_from_network_scan(self, mac_address):
         """Return the IP address associated with a MAC address.
 

=== added directory 'maastest/testing'
=== added file 'maastest/testing/__init__.py'
=== added file 'maastest/testing/factory.py'
--- maastest/testing/factory.py	1970-01-01 00:00:00 +0000
+++ maastest/testing/factory.py	2014-04-09 06:59:27 +0000
@@ -0,0 +1,33 @@
+# Copyright 2014 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Factory helpers for testing."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+str = None
+
+__metaclass__ = type
+__all__ = [
+    'make_file',
+    ]
+
+from fixtures import TempDir
+import os.path
+
+
+def make_file(testcase, name=None, content=None):
+    """Write a file.  Attach cleanup to `testcase`."""
+    if content is None:
+        content = testcase.getUniqueString()
+    testcase.assertIsInstance(content, bytes)
+    if name is None:
+        name = testcase.getUniqueString()
+    path = os.path.join(testcase.useFixture(TempDir()).path, name)
+    with open(path, 'wb') as f:
+        f.write(content)
+    return path

=== modified file 'maastest/tests/test_kvmfixture.py'
--- maastest/tests/test_kvmfixture.py	2014-02-26 21:05:58 +0000
+++ maastest/tests/test_kvmfixture.py	2014-04-09 06:59:27 +0000
@@ -1,4 +1,4 @@
-# Copyright 2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2013-2014 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for `maastest.kvmfixture`."""
@@ -13,13 +13,16 @@
 __all__ = []
 
 import itertools
+import os.path
 import random
 from random import randint
+from tempfile import gettempdir
 import textwrap
 
 import fixtures
 from lxml import etree
 from maastest import kvmfixture
+from maastest.testing.factory import make_file
 from maastest.utils import read_file
 import mock
 import netaddr
@@ -694,6 +697,34 @@
                 'uvt-kvm', 'wait', fixture.name], check_call=True)],
             kvmfixture.run_command.mock_calls)
 
+    def test_upload_file_uploads_file(self):
+        self.patch(kvmfixture, 'run_command', mock.MagicMock())
+        fixture = self.make_KVMFixture(kvm_timeout=None)
+        local_file = make_file(self)
+        remote_file = os.path.join(gettempdir(), self.getUniqueString())
+        fixture.upload_file(local_file, remote_file)
+        self.assertEqual(
+            [
+                mock.call(
+                    (
+                        ['scp'] +
+                        fixture._get_base_ssh_options() +
+                        [local_file, remote_file]
+                    ),
+                    check_call=True)
+            ],
+            kvmfixture.run_command.mock_calls)
+
+    def test_upload_file_defaults_to_base_name_in_default_dir(self):
+        self.patch(kvmfixture, 'run_command', mock.MagicMock())
+        fixture = self.make_KVMFixture(kvm_timeout=None)
+        local_file = make_file(self)
+        fixture.upload_file(local_file)
+        [command] = kvmfixture.run_command.mock_calls
+        name, args, kwargs = command
+        (command_line,) = args
+        self.assertEqual(os.path.basename(local_file), command_line[-1])
+
 
 def xml_normalize(snippet):
     parser = etree.XMLParser(remove_blank_text=True)

=== modified file 'maastest/tests/test_maasfixture.py'
--- maastest/tests/test_maasfixture.py	2014-04-03 02:10:29 +0000
+++ maastest/tests/test_maasfixture.py	2014-04-09 06:59:27 +0000
@@ -20,19 +20,18 @@
     repeat,
     )
 import json
-import os.path
 from random import randint
 from subprocess import check_call
 from textwrap import dedent
 
 from apiclient.maas_client import MAASClient
-from fixtures import TempDir
 from maastest import (
     kvmfixture,
     maasfixture,
     utils,
     )
 from maastest.maas_enums import NODEGROUPINTERFACE_MANAGEMENT
+from maastest.testing.factory import make_file
 import mock
 import netaddr
 from six import text_type
@@ -112,14 +111,7 @@
             """ % (old_url, new_url)).encode('ascii')
 
     def make_file(self, name=None, content=None):
-        """Write a file."""
-        self.assertIsInstance(content, bytes)
-        if name is None:
-            name = self.getUniqueString()
-        path = os.path.join(self.useFixture(TempDir()).path, name)
-        with open(path, 'wb') as f:
-            f.write(content)
-        return path
+        return make_file(self, name=name, content=content)
 
     def patch_MAASClient(self, client, method, content, code=httplib.OK):
         """Patch a MAASClient so that it will return a pre-cooked response."""
@@ -132,8 +124,8 @@
     def test_compose_rewrite_for_default_maas_url_rewrites_url(self):
         old_url = 'http://1.1.1.1/'
         new_url = 'http://2.2.2.2/'
-        config_file = self.make_file(
-            content=self.make_maas_local_settings(old_url))
+        config_file = make_file(
+            self, content=self.make_maas_local_settings(old_url))
 
         command = maasfixture.compose_rewrite_for_default_maas_url(
             config_file, new_url)
@@ -149,8 +141,8 @@
     def test_compose_rewrite_for_default_maas_url_accepts_path(self):
         old_url = 'http://old-host.com/OLDMAAS'
         new_url = 'http://new-host.local/NEWMAAS'
-        config_file = self.make_file(
-            content=self.make_maas_local_settings(old_url))
+        config_file = make_file(
+            self, content=self.make_maas_local_settings(old_url))
 
         check_call(
             maasfixture.compose_rewrite_for_default_maas_url(
@@ -165,7 +157,7 @@
         old_url = "http://1.1.1.1/"
         old_content = b'\n'.join(
             [top, self.make_maas_local_settings(old_url), bottom])
-        config_file = self.make_file(content=old_content)
+        config_file = make_file(self, content=old_content)
         new_url = "http://2.2.2.2/"
 
         check_call(
@@ -180,7 +172,7 @@
         top = b'#DEFAULT_MAAS_URL = "http://9.9.9.9/"\n'
         old_url = "http://1.1.1.1/"
         old_content = top + self.make_maas_local_settings(old_url)
-        config_file = self.make_file(content=old_content)
+        config_file = make_file(self, content=old_content)
         new_url = "http://2.2.2.2/"
 
         check_call(