Merge lp:~gz/juju-ci-tools/restore_assess_debug_log into lp:juju-ci-tools

[Martin Packman]

Restore r1184 without assess_log_rotation addition

Code was useful and worth having in tree, even if we don't enable the additional testing. Also includes a bug fix for get_juju_output I want.

[Curtis Hovey (sinzui)]

Thank you. I have a suggestion inline.

review: Approve (code)

[lp:~gz/juju-ci-tools/restore_assess_debug_log]

1265. By Martin Packman on 2016-02-12

Change test_debug_log as suggested by sinzui in review

=== modified file 'assess_log_rotation.py'
--- assess_log_rotation.py	2016-01-12 16:14:17 +0000
+++ assess_log_rotation.py	2016-02-12 22:33:29 +0000
@@ -32,14 +32,30 @@
         super(LogRotateError, self).__init__(message)
 
 
+def test_debug_log(client, timeout=180, lines=100):
+    """After doing log rotation, we should be able to see debug-log output."""
+    out = client.get_juju_output("debug-log", "--lines={}".format(lines),
+                                 "--limit={}".format(lines), timeout=timeout)
+    content = out.splitlines()
+    if len(content) != lines:
+        raise LogRotateError("We expected {} lines of output, got {}".format(
+            lines, len(content)))
+
+
 def test_unit_rotation(client):
     """Tests unit log rotation."""
+    # TODO: as part of testing that when a unit sending lots of logs triggers
+    # unit log rotation, we should also test that all-machines.log and future
+    # logsink.log get rotated.
+    # It would also be possible to test that the logs database doesn't grow too
+    # large.
     test_rotation(client,
                   "/var/log/juju/unit-fill-logs-0.log",
                   "unit-fill-logs-0",
                   "fill-unit",
                   "unit-size",
                   "megs=300")
+    # TODO: either call test_debug_log here or add a new assess entry for it.
 
 
 def test_machine_rotation(client):
 
=== modified file 'jujupy.py'
--- jujupy.py	2016-02-12 21:31:30 +0000
+++ jujupy.py	2016-02-12 22:33:29 +0000
@@ -438,7 +438,7 @@
             if proc.returncode != 0:
                 log.debug(sub_error)
                 e = subprocess.CalledProcessError(
-                    proc.returncode, args[0], sub_error)
+                    proc.returncode, args[0], sub_output)
                 e.stderr = sub_error
                 if (
                     'Unable to connect to environment' in sub_error or
 
=== modified file 'tests/test_assess_log_rotation.py'
--- tests/test_assess_log_rotation.py	2015-12-15 16:21:27 +0000
+++ tests/test_assess_log_rotation.py	2016-02-12 22:33:29 +0000
@@ -1,22 +1,26 @@
 from argparse import Namespace
 from contextlib import contextmanager
-from unittest import TestCase
 
-from mock import patch
+from mock import (
+    Mock,
+    patch,
+)
 
 from assess_log_rotation import (
+    check_expected_backup,
     check_for_extra_backup,
-    check_expected_backup,
     check_log0,
     LogRotateError,
     make_client_from_args,
     parse_args,
+    test_debug_log,
 )
 from jujupy import (
     EnvJujuClient,
     _temp_env as temp_env,
     yaml_loads,
     )
+from tests import TestCase
 
 good_yaml = \
     """
@@ -148,6 +152,26 @@
                 "/var/log/juju/unit-fill-logs-0.log", big_obj)
 
 
+class TestTestDebugLog(TestCase):
+
+    def test_happy_log(self):
+        client = Mock()
+        client.get_juju_output.return_value = '\n'*100
+        # Ensure that no exception is raised
+        test_debug_log(client, timeout=120)
+        client.get_juju_output.assert_called_once_with(
+            "debug-log", "--lines=100", "--limit=100", timeout=120)
+
+    def test_unhappy_log(self):
+        client = Mock()
+        client.get_juju_output.return_value = ''
+        # Ensure that no exception is raised
+        with self.assertRaises(LogRotateError):
+            test_debug_log(client)
+        client.get_juju_output.assert_called_once_with(
+            "debug-log", "--lines=100", "--limit=100", timeout=180)
+
+
 class TestParseArgs(TestCase):
 
     def test_parse_args(self):
@@ -165,12 +189,6 @@
 
 class TestMakeClientFromArgs(TestCase):
 
-    def setUp(self):
-        super(TestMakeClientFromArgs, self).setUp()
-        patcher = patch('subprocess.Popen', side_effect=Exception)
-        patcher.start()
-        self.addCleanup(patcher.stop)
-
     @contextmanager
     def make_client_cxt(self):
         with temp_env({'environments': {'foo': {}}}):
 
=== modified file 'tests/test_jujupy.py'
--- tests/test_jujupy.py	2016-02-12 21:31:30 +0000
+++ tests/test_jujupy.py	2016-02-12 22:33:29 +0000
@@ -2591,12 +2591,13 @@
 
     def test_get_juju_output_stderr(self):
         env = SimpleEnvironment('foo')
-        fake_popen = FakePopen(None, 'Hello!', 1)
+        fake_popen = FakePopen('Hello', 'Error!', 1)
         client = EnvJujuClient1X(env, None, None)
         with self.assertRaises(subprocess.CalledProcessError) as exc:
             with patch('subprocess.Popen', return_value=fake_popen):
                 client.get_juju_output('bar')
-        self.assertEqual(exc.exception.stderr, 'Hello!')
+        self.assertEqual(exc.exception.output, 'Hello')
+        self.assertEqual(exc.exception.stderr, 'Error!')
 
     def test_get_juju_output_accepts_timeout(self):
         env = SimpleEnvironment('foo')