Merge lp:~rvb/maas-test/add-log-files into lp:maas-test

[Raphaël Badin]

Include logs in the report when tests fail.

When tests fail, having the logs will help us debug issues.

The changes to test_import_maas_images_imports_pxe_images are nothing but drive-by fixes.

[Jeroen T. Vermeulen (jtv)]

Nice. This'll help. Ingenious trick for getting the files into the log... Not the prettiest presentation perhaps, but it'll get the job done.

FWIW the check_call parameter to the run_command functions defaults to False, so if you want to avoid breaking up that line in collect_logs, that might help. Not that I'm saying you should; you may have left it deliberately explicit.

review: Approve

[Raphaël Badin (rvb)]

> Nice. This'll help. Ingenious trick for getting the files into the log...
> Not the prettiest presentation perhaps, but it'll get the job done.
>
> FWIW the check_call parameter to the run_command functions defaults to False,
> so if you want to avoid breaking up that line in collect_logs, that might
> help. Not that I'm saying you should; you may have left it deliberately
> explicit.

Yes, it's deliberately explicit. I set it to Fasle because I don't want the execution to stop if a file doesn't exist.

=== modified file 'maastest/maasfixture.py'
--- maastest/maasfixture.py	2013-11-21 13:33:05 +0000
+++ maastest/maasfixture.py	2013-11-22 10:41:25 +0000
@@ -56,6 +56,23 @@
         config_file,
         ]
 
+# Log files that are interesting when debugging MAAS issues.  They will
+# be collected just before the fixture is disposed of.
+LOG_FILES = [
+    # Syslog contains DHCP requests.
+    '/var/log/syslog',
+    # DHCP lease file.
+    '/var/lib/maas/dhcp/dhcpd.leases',
+    # MAAS logs.
+    '/var/log/maas/maas.log',
+    '/var/log/maas/pserv.log',
+    '/var/log/maas/celery-region.log',
+    '/var/log/maas/celery.log',
+    # Apache logs.
+    '/var/log/apache2/access.log',
+    '/var/log/apache2/error.log',
+]
+
 
 class MAASFixture(Fixture):
     """A fixture for a MAAS server."""
@@ -176,6 +193,17 @@
         if self.kvm_fixture.direct_ip is not None:
             self.check_cluster_connected()
             self.configure_cluster()
+        # Cleanups registration.
+        self.addCleanup(self.collect_logs)
+
+    def collect_logs(self):
+        for filename in LOG_FILES:
+            # The result of every command run on the VM goes through
+            # testtools.TestCase.addDetail() so we just need to
+            # 'cat' the files to get their content included in the
+            # report.
+            self.kvm_fixture.run_command([
+                'sudo', 'cat', filename], check_call=False)
 
     def get_maas_api_client(self, api_key):
         """Create and return a MAASClient.
 
=== modified file 'maastest/main.py'
--- maastest/main.py	2013-11-21 13:39:24 +0000
+++ maastest/main.py	2013-11-22 10:41:25 +0000
@@ -48,11 +48,13 @@
             cls.maas = MAASFixture(cls.machine)
             cls.maas.setUp()
         except:
+            cls.maas.cleanUp()
             cls.machine.cleanUp()
             raise
 
     @classmethod
     def tearDownClass(cls):
+        cls.maas.cleanUp()
         cls.machine.cleanUp()
         super(TestMAAS, cls).tearDownClass()
 
 
=== modified file 'maastest/tests/test_maasfixture.py'
--- maastest/tests/test_maasfixture.py	2013-11-21 13:33:05 +0000
+++ maastest/tests/test_maasfixture.py	2013-11-22 10:41:25 +0000
@@ -335,6 +335,19 @@
             ),
             (call_mock.mock_calls, put_mock.mock_calls))
 
+    def test_collect_logs_collects_logs(self):
+        kvm_fixture = self.make_kvm_fixture()
+        maas_fixture = maasfixture.MAASFixture(kvm_fixture)
+
+        maas_fixture.collect_logs()
+
+        expected_calls = [
+            mock.call(['sudo', 'cat', filename], check_call=False)
+            for filename in maasfixture.LOG_FILES]
+        self.assertEqual(
+            expected_calls,
+            kvm_fixture.run_command.mock_calls)
+
     def test_setUp_sets_admin_details(self):
         fixture = self.make_kvm_fixture()
         maas_fixture = maasfixture.MAASFixture(fixture)
@@ -364,6 +377,26 @@
                 client_key,
             ))
 
+    def test_setUp_calls_cleanup_methods(self):
+        fixture = self.make_kvm_fixture()
+        maas_fixture = maasfixture.MAASFixture(fixture)
+        self.patch(
+            maas_fixture, 'query_api_key',
+            mock.MagicMock(return_value='fake:api:key'))
+        self.patch(
+            maas_fixture, 'collect_logs', mock.MagicMock())
+
+        with maas_fixture:
+            pass
+
+        self.assertEqual(
+            [
+                [mock.call()],
+            ],
+            [
+                maas_fixture.collect_logs.mock_calls,
+            ])
+
     def test_setUp_calls_methods_if_direct_ip_set(self):
         fixture = self.make_kvm_fixture()
         fixture.direct_ip = '127.11.24.38'
@@ -419,16 +452,15 @@
                 maas_fixture.configure_cluster.mock_calls,
             ])
 
-    def test_import_mmas_images_imports_pxe_images(self):
+    def test_import_maas_images_imports_pxe_images(self):
         kvm_fixture = self.make_kvm_fixture()
-        self.patch(kvm_fixture, 'run_command', mock.MagicMock())
         maas_fixture = maasfixture.MAASFixture(kvm_fixture)
         maas_fixture.import_maas_images()
         self.assertEqual(
             mock.call([
-                u'sudo',
-                u'RELEASES=precise',
-                u'ARCHES=amd64/generic i386/generic',
-                u'maas-import-pxe-files',
+                'sudo',
+                'RELEASES=precise',
+                'ARCHES=amd64/generic i386/generic',
+                'maas-import-pxe-files',
             ], check_call=True),
             kvm_fixture.run_command.mock_calls[-1])