Merge lp:~benji/landscape-client/better-self-signed-cert-ux-3 into lp:~landscape/landscape-client/trunk

[Benji York]

This branch is a pure refactoring of the way the client configuration
communicates so as to separate the UI and network code in preparation
for better SSL error handling and user interaction.

Tests were then able to be improved -- but not as far as we would like.

Notes from Chris:

Most of the code breaks out nested function declaration to make them
easier to test, using functools.partial to keep the invocation sane in a
twisted context (test them "flat", use functools to pass closures to
event handlers).

We used manual stubs instead of mocks for readability and personal
preference [of both of us].

This branch is a pure refactoring of the way the client configuration communicates so as to separate the UI and network code in preparation for better SSL error handling and user interaction.

Tests were then able to be improved -- but not as far as we would like.

Notes from Chris:

Most of the code breaks out nested function declaration to make them easier to test, using functools.partial to keep the invocation sane in a twisted context (test them "flat", use functools to pass closures to event handlers).

We used manual stubs instead of mocks for readability and personal preference (of both of us).

[Chris Glass (tribaal)]

Looks good, and seems to work.

The merge description is wrong however - this branch "only" refactors the current code to make it easier to grok and test, but there is currently no functional improvement (that should come in a subsequent branch).

Note (can be used in the change description):
Most of the code breaks out nested function declaration to make them easier to test, using functools.partial to keep the invocation sane in a twisted context (test them "flat", use functools to pass closures to event handlers).

Note 2:
We used manual stubs instead of mocks for readability and personal preference (of both of us).

I'll comment again when I will have performed actual manual tests (give me a couple of hours :) )

[Benji York (benji)]

> Looks good, and seems to work.
>
> The merge description is wrong however

I've fixed the MP description and incorporated your notes below.

> I'll comment again when I will have performed actual manual tests (give me a
> couple of hours :) )

/me crosses fingers.

[Chris Glass (tribaal)]

Summary testing succeeded.

+1

review: Approve

[Данило Шеган (danilo)]

Seems to work fine against my self-signed Landscape. A few comments inline.

I am not too happy with the tests, since you seem to be mostly testing the wiring, and never that it actually works well together. I do understand that coming up with proper tests would require even more work, and with the understanding that this does not make the code base any worse, I am approving it.

Finally, I am not sure it's such a great idea to have Chris do one of the reviews _if_ he has contributed a lot of code/opinions while you were developing this branch. But you guys know best how much his mind is already "tainted" with the approach :)

review: Approve

[Chris Glass (tribaal)]

I will abstain from this one since I peer-programmed some of it.

review: Abstain

[Chris Glass (tribaal)]

Good point Danilo, I added another review slot and abstained.

[Björn Tillenius (bjornt)]

Why couldn't this be broken up into smaller branches?

[Free Ekanayaka (free.ekanayaka)]

> Why couldn't this be broken up into smaller branches?

Agreed. I didn't look at the code in detail, but I was expecting to see at least 2 branches: one for the pure refactoring of the registration code and one for adding the new functionality.

[Chris Glass (tribaal)]

Free, this *is* the pure refactoring branch.

[Björn Tillenius (bjornt)]

> Free, this *is* the pure refactoring branch.

In that case, the MP description is wrong. It says that more info is provided to the user and more error conditions are handled.

[Benji York (benji)]

> > Free, this *is* the pure refactoring branch.
>
> In that case, the MP description is wrong. It says that more info is provided
> to the user and more error conditions are handled.

I had some sort of edit failure and didn't get the MP description corrected. It is right now.

[Benji York (benji)]

> Why couldn't this be broken up into smaller branches?

It surely could have been, but given the nigh untestable nature of the current code structure (even after our refactoring), one large branch was the least risky way to proceed without undue exploratory testing, especially given Chris' upcoming disconnectedness.

[Benji York (benji)]

> Seems to work fine against my self-signed Landscape. A few comments inline.
>
> I am not too happy with the tests

As are we. Unfortunately, better tests would have required much more refactoring as well as having some place to put integration tests, which were expressly forbid when the idea of adding some to the existing test base was floated.

> Finally, I am not sure it's such a great idea to have Chris do one of the
> reviews _if_ he has contributed a lot of code/opinions while you were
> developing this branch.

Christ has recused himself from the review.

(I am of the opinion that his review would have been fine, given that I am not big on two-person reviews in the first place, and more so if a branch is the result of pair programming -- which this was.)

[Данило Шеган (danilo)]

Yes, agreed re better tests: I don't think it's worth it.

I am happy about the Christ not having an opinion on this branch :)

As for Chris, pair programming usually does lead to better code, but at the same time can result in two minds agreeing to decisions and never re-questioning them after the original agreement (I've been guilty of the same thing).

Anyway, my overall assesement is that this is improvement over the existing code (and tests are no worse) which is reflected in my approval :)

[Benji York (benji)]

Thanks for the good review Danilo. I fixed the things you pointed out inline. I also noticed some of the pre-existing tests were generating output to stdout, so I corrected those.

[Björn Tillenius (bjornt)]

Sorry, I won't have time to finish this review before my EOD. I added some initial comments inline.

[Björn Tillenius (bjornt)]

Ok, I have had time to look at this branch more closely now. How would you feel if I put it in Needs Fixing and requested you to rewrite the tests?

I understand that it's some work doing that, and that fakes are a small improvement over mocking, but the emphasis is on small here. The way you use fakes makes it very similar to mocking. It's still easier to read, but you're still testing the that the code is written in the same way the tests expects them to be.

I would rather that the tests were a bit more functional, in that you set up the conditions, then call register() and check what it returns. I don't care how the deferreds are set up internally. What I care about is that register() returns "success" when the registration succeeds.

It's not quite trivial, but here's an example of a test that checks that register() returns "success" when the registration is successful:

  https://pastebin.canonical.com/126660/

What do you think?

review: Needs Information

[Benji York (benji)]

> What do you think?

I haven't taken the time to look deeply into your proposed test approach, but I agree with everything you said. Unfortunately, the amount of time invested in this branch thus far means that the size of the marginal improvement must be very large for it to be worth it and regrettably, that's not the case.

I plan on keeping the test approach in mind for the next time we work on this code (which I hope is soon, it needs quite a bit of help).

Thank you for the review.

[Björn Tillenius (bjornt)]

Ok, I won't push too hard on adding better tests, since the existing tests weren't great either. But the old tests did actually tests that the async machinery really works, which your tests don't. So I would like you to at least add the test that I pasted, so you have one test that does end-to-end testing.

I'd also like you to respond to the inline comments I added here, as well as those I added previously, and push the latest revision please. I still don't see the changes you did in response to Danilo's comments.

[lp:~benji/landscape-client/better-self-signed-cert-ux-3]

827. By Benji York on 2015-02-26

docstring all the things

828. By Benji York on 2015-02-26

remove duplicate and redundant assertion

829. By Benji York on 2015-02-26

- add tests for reporting registration result to the user
- fix up tests that print to stdout so they won't

[Benji York (benji)]

> [...] So I would like you to at
> least add the test that I pasted, so you have one test that does end-to-end
> testing.

Done. (BTW, that diff was odd. I'm curious how you generated it. I had to hand apply it.)

> I'd also like you to respond to the inline comments I added here, as well as
> those I added previously

Done.

> and push the latest revision please.

Done.

[Björn Tillenius (bjornt)]

Sorry for being a pain :) But the latest diff isn't what I would expect it to be. It lacks at least one change that you said you would do (I added an inline comment), and the test I added isn't there. Instead you added a bunch of other tests that do mocking and seem redundant?

BTW, maybe the patch was odd because you looked at the wrong pastebin? ;) Although, the patch could look odd since I based it on what you pushed. Actually, looking at what you pushed, it's quite clear that you didn't push the changes you did in reply to my comments. You pushed revisions from Feb 26.

Please make it a habit to always push the latest code to LP. It's a bit complicated to review the branch when you know that there are other changes that you can't see.

[lp:~benji/landscape-client/better-self-signed-cert-ux-3]

830. By Benji York on 2015-03-04

- add a test that runs register() in a more full-boddied way
- use Twisted parameter passing functionality instead of partials
  - tweak parameter order to account for different calling style
- add and improve motivational comments around test assertions
- fix a test assertion that was unreachable

[Benji York (benji)]

> Sorry for being a pain :) But the latest diff isn't what I would expect it to
> be.

I pushed, but apparently forgot to commit beforehand.

> BTW, maybe the patch was odd because you looked at the wrong pastebin? ;)

This is the pastebin I was referring to: https://pastebin.canonical.com/126660/

The oddities I see are the blank lines with a dot in the indicator position and the lack of whitespace after the file name in the headers.

[Björn Tillenius (bjornt)]

Ok, let's get this branch landed now!

> This is the pastebin I was referring to: https://pastebin.canonical.com/126660/
>
> The oddities I see are the blank lines with a dot in the indicator position and the lack of
> whitespace after the file name in the headers.

Oh, that is odd indeed. I've never seen that before, but it seems to be since I copied
it from 'bzr diff | vim -' and the diff highlighter adds those periods.

review: Approve

[lp:~benji/landscape-client/better-self-signed-cert-ux-3]

831. By Benji York on 2015-03-04

merge from trunk

=== modified file 'landscape/broker/amp.py'
--- landscape/broker/amp.py	2015-01-12 12:10:05 +0000
+++ landscape/broker/amp.py	2015-03-04 14:53:14 +0000
@@ -65,6 +65,21 @@
             return maybeDeferred(callable, *args)
         return succeed(None)
 
+    def call_on_event(self, handlers):
+        """Call a given handler as soon as a certain event occurs.
+
+        @param handlers: A dictionary mapping event types to callables, where
+            an event type is string (the name of the event). When the first of
+            the given event types occurs in the broker reactor, the associated
+            callable will be fired.
+        """
+        result = self.broker_server.listen_events(handlers.keys())
+        return result.addCallback(
+            lambda (event_type, kwargs): handlers[event_type](**kwargs))
+
+    def register(self):
+        return succeed(None)
+
 
 class RemoteBrokerConnector(ComponentConnector):
     """Helper to create connections with the L{BrokerServer}."""

=== modified file 'landscape/configuration.py'
--- landscape/configuration.py	2015-01-12 09:27:02 +0000
+++ landscape/configuration.py	2015-03-04 14:53:14 +0000
@@ -4,12 +4,14 @@
 for the C{landscape-config} script.
 """
 
+from __future__ import print_function
+
+from functools import partial
 import base64
-import time
-import sys
+import getpass
 import os
-import getpass
 import pwd
+import sys
 
 from StringIO import StringIO
 
@@ -35,6 +37,8 @@
 
 
 def print_text(text, end="\n", error=False):
+    """Display the given text to the user, using stderr if flagged as an error.
+    """
     if error:
         stream = sys.stderr
     else:
@@ -584,8 +588,60 @@
     return key_filename
 
 
-def register(config, on_message=print_text, on_error=sys.exit, reactor=None,
-             max_retries=14):
+def failure(add_result):
+    """Handle a failed communication by recording the kind of failure."""
+    add_result("failure")
+
+
+def exchange_failure(add_result, ssl_error=False):
+    """Handle a failed call by recording if the failure was SSL-related."""
+    if ssl_error:
+        add_result("ssl-error")
+    else:
+        add_result("non-ssl-error")
+
+
+def handle_registration_errors(add_result, connector, failure):
+    """HAndle a failed registration by recording the kind of failure."""
+    # We'll get invalid credentials through the signal.
+    failure.trap(InvalidCredentialsError, MethodCallError)
+    add_result("registration-error")
+    connector.disconnect()
+
+
+def success(add_result):
+    """Handle a successful communication by recording the fact."""
+    add_result("success")
+
+
+def done(ignored_result, connector, reactor):
+    """Clean up after communicating with the server."""
+    connector.disconnect()
+    reactor.stop()
+
+
+def got_connection(add_result, connector, reactor, remote):
+    """Handle becomming connected to a broker."""
+    handlers = {"registration-done": partial(success, add_result),
+                "registration-failed": partial(failure, add_result),
+                "exchange-failed": partial(exchange_failure, add_result)}
+    deferreds = [
+        remote.call_on_event(handlers),
+        remote.register().addErrback(
+            handle_registration_errors, add_result, connector)]
+    results = gather_results(deferreds)
+    results.addCallback(done, connector, reactor)
+    return results
+
+
+def got_error(failure, print=print):
+    """...from broker."""
+    print(failure.getTraceback(), file=sys.stderr)
+    raise SystemExit
+
+
+def register(config, reactor, connector_factory=RemoteBrokerConnector,
+        got_connection=got_connection, max_retries=14, results=None):
     """Instruct the Landscape Broker to register the client.
 
     The broker will be instructed to reload its configuration and then to
@@ -605,91 +661,48 @@
         max_retries = 14
 
         0.05 * (1 - 1.62 ** 14) / (1 - 1.62) = 69 seconds
-   """
-    if reactor is None:
-        reactor = LandscapeReactor()
-    exit_with_error = []
-
-    def stop(errors):
-        if not config.ok_no_register:
-            for error in errors:
-                if error is not None:
-                    exit_with_error.append(error)
-        connector.disconnect()
-        reactor.stop()
-
-    def failure():
-        on_message("Invalid account name or "
-                   "registration key.", error=True)
-        return 2
-
-    def success():
-        on_message("System successfully registered.")
-
-    def exchange_failure(ssl_error=False):
-        if ssl_error:
-            message = ("\nThe server's SSL information is incorrect, or fails "
-                       "signature verification!\n"
-                       "If the server is using a self-signed certificate, "
-                       "please ensure you supply it with the --ssl-public-key "
-                       "parameter.")
-        else:
-            message = ("\nWe were unable to contact the server.\n"
-                       "Your internet connection may be down. "
-                       "The landscape client will continue to try and contact "
-                       "the server periodically.")
-
-        on_message(message, error=True)
-        return 2
-
-    def handle_registration_errors(failure):
-        # We'll get invalid credentials through the signal.
-        failure.trap(InvalidCredentialsError, MethodCallError)
-        connector.disconnect()
-
-    def catch_all(failure):
-        on_message(failure.getTraceback(), error=True)
-        on_message("Unknown error occurred.", error=True)
-        return [2]
-
-    on_message("Please wait... ", "")
-
-    time.sleep(2)
-
-    def got_connection(remote):
-        handlers = {"registration-done": success,
-                    "registration-failed": failure,
-                    "exchange-failed": exchange_failure}
-        deferreds = [
-            remote.call_on_event(handlers),
-            remote.register().addErrback(handle_registration_errors)]
-        # We consume errors here to ignore errors after the first one.
-        # catch_all will be called for the very first deferred that fails.
-        results = gather_results(deferreds, consume_errors=True)
-        results.addErrback(catch_all)
-        results.addCallback(stop)
-
-    def got_error(failure):
-        on_message("There was an error communicating with the Landscape"
-                   " client.", error=True)
-        on_message("This machine will be registered with the provided "
-                   "details when the client runs.", error=True)
-        stop([2])
-
-    connector = RemoteBrokerConnector(reactor, config)
-    result = connector.connect(max_retries=max_retries, quiet=True)
-    result.addCallback(got_connection)
-    result.addErrback(got_error)
-
+    """
+    if results is None:
+        results = []
+    add_result = results.append
+
+    connector = connector_factory(reactor, config)
+    connection = connector.connect(max_retries=max_retries, quiet=True)
+    connection.addCallback(
+        partial(got_connection, add_result, connector, reactor))
+    connection.addErrback(got_error)
     reactor.run()
 
-    if exit_with_error:
-        on_error(exit_with_error[0])
-
-    return result
-
-
-def main(args):
+    assert len(results) == 1, "We expect exactly one result."
+    # Results will be things like "success" or "ssl-error".
+    return results[0]
+
+
+def report_registration_outcome(what_happened, print=print):
+    """
+    Report the registrtion interaction outcome to the user in human-readable
+    form.
+    """
+    if what_happened == "success":
+        print("System successfully registered.")
+    elif what_happened == "failure":
+        print("Invalid account name or registration key.", file=sys.stderr)
+    elif what_happened == "ssl-error":
+        print("\nThe server's SSL information is incorrect, or fails "
+              "signature verification!\n"
+              "If the server is using a self-signed certificate, "
+              "please ensure you supply it with the --ssl-public-key "
+              "parameter.", file=sys.stderr)
+    elif what_happened == "non-ssl-error":
+        print("\nWe were unable to contact the server.\n"
+              "Your internet connection may be down. "
+              "The landscape client will continue to try and contact "
+              "the server periodically.", file=sys.stderr)
+
+
+def main(args, print=print):
+    """Interact with the user and the server to set up client configuration."""
+
     config = LandscapeSetupConfiguration()
     try:
         config.load(args)
@@ -716,11 +729,16 @@
         print_text(str(e))
         sys.exit("Aborting Landscape configuration")
 
+    print("Please wait...")
+
     # Attempt to register the client.
+    reactor = LandscapeReactor()
     if config.silent:
-        register(config)
+        result = register(config, reactor)
+        report_registration_outcome(result, print=print)
     else:
         answer = raw_input("\nRequest a new registration for "
                            "this computer now? (Y/n): ")
         if not answer.upper().startswith("N"):
-            register(config)
+            result = register(config, reactor)
+            report_registration_outcome(result, print=print)

=== modified file 'landscape/tests/helpers.py'
--- landscape/tests/helpers.py	2013-09-25 08:43:17 +0000
+++ landscape/tests/helpers.py	2015-03-04 14:53:14 +0000
@@ -359,6 +359,7 @@
             transport_factory = FakeTransport
 
         test_case.broker_service = FakeBrokerService(config)
+        test_case.reactor = test_case.broker_service.reactor
         test_case.remote = FakeRemoteBroker(
             test_case.broker_service.exchanger,
             test_case.broker_service.message_store,

=== modified file 'landscape/tests/test_configuration.py'
--- landscape/tests/test_configuration.py	2015-01-12 09:29:17 +0000
+++ landscape/tests/test_configuration.py	2015-03-04 14:53:14 +0000
@@ -1,27 +1,29 @@
+from __future__ import print_function
+
+from ConfigParser import ConfigParser
+from cStringIO import StringIO
+from getpass import getpass
 import os
 import sys
-from getpass import getpass
-from ConfigParser import ConfigParser
-from cStringIO import StringIO
-
-from twisted.internet.defer import succeed, fail
-from twisted.internet.task import Clock
-
-from landscape.lib.amp import MethodCallSender
-from landscape.reactor import LandscapeReactor, FakeReactor
+import unittest
+
+from twisted.internet.defer import succeed
+
+from landscape.broker.registration import InvalidCredentialsError
+from landscape.tests.helpers import FakeBrokerServiceHelper
+from landscape.broker.tests.helpers import RemoteBrokerHelper
+from landscape.lib.amp import MethodCallError
 from landscape.lib.fetch import HTTPCodeError, PyCurlError
 from landscape.configuration import (
     print_text, LandscapeSetupScript, LandscapeSetupConfiguration,
     register, setup, main, setup_init_script_and_start_client,
     stop_client_and_disable_init_script, ConfigurationError,
     ImportOptionError, store_public_key_data,
-    bootstrap_tree)
-from landscape.broker.registration import InvalidCredentialsError
+    bootstrap_tree, got_connection, success, failure, exchange_failure,
+    handle_registration_errors, done, got_error, report_registration_outcome)
 from landscape.sysvconfig import SysVConfig, ProcessError
-from landscape.tests.helpers import (
-    LandscapeTest, BrokerServiceHelper, EnvironSaverHelper)
-from landscape.tests.mocker import ARGS, ANY, MATCH, CONTAINS, expect
-from landscape.broker.amp import RemoteBroker
+from landscape.tests.helpers import LandscapeTest, EnvironSaverHelper
+from landscape.tests.mocker import ANY, MATCH, CONTAINS, expect
 
 
 class LandscapeConfigurationTest(LandscapeTest):
@@ -41,6 +43,113 @@
         return config
 
 
+class SuccessTests(unittest.TestCase):
+    def test_success(self):
+        """The success handler records the success."""
+        results = []
+        success(results.append)
+        self.assertEqual(["success"], results)
+
+
+class FailureTests(unittest.TestCase):
+    def test_failure(self):
+        """The failure handler records the failure and returns non-zero."""
+        results = []
+        self.assertNotEqual(0, failure(results.append))
+        self.assertEqual(["failure"], results)
+
+
+class ExchangeFailureTests(unittest.TestCase):
+
+    def test_exchange_failure_ssl(self):
+        """The exchange_failure() handler records whether or not the failure
+        involved SSL or not and returns non-zero."""
+        results = []
+        self.assertNotEqual(0,
+            exchange_failure(results.append, ssl_error=True))
+        self.assertEqual(["ssl-error"], results)
+
+    def test_exchange_failure_non_ssl(self):
+        """
+        The exchange_failure() handler records whether or not the failure
+        involved SSL or not and returns non-zero.
+        """
+        results = []
+        self.assertNotEqual(0,
+            exchange_failure(results.append, ssl_error=False))
+        self.assertEqual(["non-ssl-error"], results)
+
+
+class HandleRegistrationErrorsTests(unittest.TestCase):
+
+    def test_handle_registration_errors(self):
+        """
+        The handle_registration_errors() function handles
+        InvalidCredentialsError and MethodCallError errors and records the
+        type of failure and disconnects.
+        """
+        class FauxFailure(object):
+            def trap(self, *trapped):
+                self.trapped_exceptions = trapped
+
+        faux_connector = FauxConnector()
+        faux_failure = FauxFailure()
+
+        results = []
+        self.assertNotEqual(0,
+            handle_registration_errors(
+                results.append, faux_connector, faux_failure))
+        self.assertEqual(["registration-error"], results)
+        self.assertTrue(faux_connector.was_disconnected)
+        self.assertTrue(
+            [InvalidCredentialsError, MethodCallError],
+            faux_failure.trapped_exceptions)
+
+
+class DoneTests(unittest.TestCase):
+
+    def test_done(self):
+        """The done() function handles cleaning up."""
+        class FauxConnector(object):
+            was_disconnected = False
+
+            def disconnect(self):
+                self.was_disconnected = True
+
+        class FauxReactor(object):
+            was_stopped = False
+
+            def stop(self):
+                self.was_stopped = True
+
+        faux_connector = FauxConnector()
+        faux_reactor = FauxReactor()
+
+        done(None, faux_connector, faux_reactor)
+        self.assertTrue(faux_connector.was_disconnected)
+        self.assertTrue(faux_reactor.was_stopped)
+
+
+class GotErrorTests(unittest.TestCase):
+
+    def test_got_error(self):
+        """The got_error() function handles displaying errors and exiting."""
+        class FauxFailure(object):
+
+            def getTraceback(self):
+                return "traceback"
+
+        printed = []
+
+        def faux_print(text, file):
+            printed.append((text, file))
+
+        with self.assertRaises(SystemExit):
+            got_error(FauxFailure(), print=faux_print)
+
+        self.assertEqual([('traceback', sys.stderr)], printed)
+
+
 class PrintTextTest(LandscapeTest):
 
     def test_default(self):
@@ -683,6 +792,11 @@
         self.assertTrue(os.path.isdir(annotations_path))
 
 
+def noop_print(*args, **kws):
+    """A print that doesn't do anything."""
+    pass
+
+
 class ConfigurationFunctionsTest(LandscapeConfigurationTest):
 
     helpers = [EnvironSaverHelper]
@@ -1022,12 +1136,12 @@
 
         # This must not be called.
         register_mock = self.mocker.replace(register, passthrough=False)
-        register_mock(ANY)
+        register_mock(ANY, ANY)
         self.mocker.count(0)
 
         self.mocker.replay()
 
-        main(["-c", self.make_working_config()])
+        main(["-c", self.make_working_config()], print=noop_print)
 
     def test_main_silent(self):
         """
@@ -1038,7 +1152,7 @@
         setup_mock(ANY)
 
         register_mock = self.mocker.replace(register, passthrough=False)
-        register_mock(ANY)
+        register_mock(ANY, ANY)
         self.mocker.count(1)
 
         self.mocker.replay()
@@ -1049,7 +1163,121 @@
             "account_name = Old Name\n"
             "registration_key = Old Password\n"
             )
-        main(["-c", config_filename, "--silent"])
+        main(["-c", config_filename, "--silent"], print=noop_print)
+
+    def test_main_user_interaction_success(self):
+        """The successful result of register() is communicated to the user."""
+        setup_mock = self.mocker.replace(setup)
+        setup_mock(ANY)
+
+        raw_input_mock = self.mocker.replace(raw_input)
+        raw_input_mock("\nRequest a new registration for "
+                       "this computer now? (Y/n): ")
+        self.mocker.result("y")
+
+        # The register() function will be called.
+        register_mock = self.mocker.replace(register, passthrough=False)
+        register_mock(ANY, ANY)
+        self.mocker.result("success")
+
+        self.mocker.replay()
+
+        printed = []
+
+        def faux_print(string, file=sys.stdout):
+            printed.append((string, file))
+
+        main(["-c", self.make_working_config()], print=faux_print)
+        self.assertEqual(
+            [("Please wait...", sys.stdout),
+             ("System successfully registered.", sys.stdout)],
+            printed)
+
+    def test_main_user_interaction_failure(self):
+        """The failed result of register() is communicated to the user."""
+        setup_mock = self.mocker.replace(setup)
+        setup_mock(ANY)
+
+        raw_input_mock = self.mocker.replace(raw_input)
+        raw_input_mock("\nRequest a new registration for "
+                       "this computer now? (Y/n): ")
+        self.mocker.result("y")
+
+        # The register() function will be called.
+        register_mock = self.mocker.replace(register, passthrough=False)
+        register_mock(ANY, ANY)
+        self.mocker.result("failure")
+
+        self.mocker.replay()
+
+        printed = []
+
+        def faux_print(string, file=sys.stdout):
+            printed.append((string, file))
+
+        main(["-c", self.make_working_config()], print=faux_print)
+        # Note that the error is output via sys.stderr.
+        self.assertEqual(
+            [("Please wait...", sys.stdout),
+             ("Invalid account name or registration key.", sys.stderr)],
+            printed)
+
+    def test_main_user_interaction_success_silent(self):
+        """A successful result is communicated to the user even with --silent.
+        """
+        setup_mock = self.mocker.replace(setup)
+        setup_mock(ANY)
+
+        raw_input_mock = self.mocker.replace(raw_input)
+        raw_input_mock(ANY)
+        self.mocker.count(0)
+
+        # The register() function will be called.
+        register_mock = self.mocker.replace(register, passthrough=False)
+        register_mock(ANY, ANY)
+        self.mocker.result("success")
+
+        self.mocker.replay()
+
+        printed = []
+
+        def faux_print(string, file=sys.stdout):
+            printed.append((string, file))
+
+        main(["--silent", "-c", self.make_working_config()], print=faux_print)
+        self.assertEqual(
+            [("Please wait...", sys.stdout),
+             ("System successfully registered.", sys.stdout)],
+            printed)
+
+    def test_main_user_interaction_failure_silent(self):
+        """A failure result is communicated to the user even with --silent.
+        """
+        setup_mock = self.mocker.replace(setup)
+        setup_mock(ANY)
+
+        raw_input_mock = self.mocker.replace(raw_input)
+        raw_input_mock(ANY)
+        self.mocker.count(0)
+
+        # The register() function will be called.
+        register_mock = self.mocker.replace(register, passthrough=False)
+        register_mock(ANY, ANY)
+        self.mocker.result("failure")
+
+        self.mocker.replay()
+
+        printed = []
+
+        def faux_print(string, file=sys.stdout):
+            printed.append((string, file))
+
+        main(["--silent", "-c", self.make_working_config()], print=faux_print)
+        # Note that the error is output via sys.stderr.
+        self.assertEqual(
+            [("Please wait...", sys.stdout),
+             ("Invalid account name or registration key.", sys.stderr)],
+            printed)
 
     def make_working_config(self):
         return self.makeFile("[client]\n"
@@ -1081,10 +1309,10 @@
         self.mocker.result("")
 
         register_mock = self.mocker.replace(register, passthrough=False)
-        register_mock(ANY)
+        register_mock(ANY, ANY)
 
         self.mocker.replay()
-        main(["--config", self.make_working_config()])
+        main(["--config", self.make_working_config()], print=noop_print)
 
     def test_errors_from_restart_landscape(self):
         """
@@ -1141,10 +1369,10 @@
         self.mocker.result("")
 
         register_mock = self.mocker.replace(register, passthrough=False)
-        register_mock(ANY)
+        register_mock(ANY, ANY)
 
         self.mocker.replay()
-        main(["-c", self.make_working_config()])
+        main(["-c", self.make_working_config()], print=noop_print)
 
     def test_setup_init_script_and_start_client(self):
         sysvconfig_mock = self.mocker.patch(SysVConfig)
@@ -1178,11 +1406,11 @@
         # The registration logic should be called and passed the configuration
         # file.
         register_mock = self.mocker.replace(register, passthrough=False)
-        register_mock(ANY)
+        register_mock(ANY, ANY)
 
         self.mocker.replay()
 
-        main(["--silent", "-c", self.make_working_config()])
+        main(["--silent", "-c", self.make_working_config()], print=noop_print)
 
     def test_disable(self):
         stop_client_and_disable_init_script_mock = self.mocker.replace(
@@ -1672,469 +1900,203 @@
                   # we care about are done.
 
 
+class FakeConnectorFactory(object):
+
+    def __init__(self, remote):
+        self.remote = remote
+
+    def __call__(self, reactor, config):
+        self.reactor = reactor
+        self.config = config
+        return self
+
+    def connect(self, max_retries=None, quiet=False):
+        return succeed(self.remote)
+
+    def disconnect(self):
+        return succeed(None)
+
+
+class RegisterRealFunctionTest(LandscapeConfigurationTest):
+
+    helpers = [FakeBrokerServiceHelper]
+
+    def setUp(self):
+        super(RegisterRealFunctionTest, self).setUp()
+        self.config = LandscapeSetupConfiguration()
+        self.config.load(["-c", self.config_filename])
+
+    def test_register_success(self):
+        self.reactor.call_later(0, self.reactor.fire, "registration-done")
+        connector_factory = FakeConnectorFactory(self.remote)
+        result = register(
+            self.config, self.reactor, connector_factory, max_retries=99)
+        self.assertEqual("success", result)
+
+
+class FauxConnection(object):
+    def __init__(self):
+        self.callbacks = []
+        self.errbacks = []
+
+    def addCallback(self, func, *args, **kws):
+        self.callbacks.append(func)
+
+    def addErrback(self, func, *args, **kws):
+        self.errbacks.append(func)
+
+
+class FauxConnector(object):
+
+    was_disconnected = False
+
+    def __init__(self, reactor=None, config=None):
+        self.reactor = reactor
+        self.config = config
+
+    def connect(self, max_retries, quiet):
+        self.max_retries = max_retries
+        self.connection = FauxConnection()
+        return self.connection
+
+    def disconnect(self):
+        self.was_disconnected = True
+
+
 class RegisterFunctionTest(LandscapeConfigurationTest):
 
-    helpers = [BrokerServiceHelper]
+    helpers = [RemoteBrokerHelper]
 
     def setUp(self):
         super(RegisterFunctionTest, self).setUp()
         self.config = LandscapeSetupConfiguration()
         self.config.load(["-c", self.config_filename])
 
-    def test_register_success(self):
-        service = self.broker_service
-
-        registration_mock = self.mocker.replace(service.registration)
-        print_text_mock = self.mocker.replace(print_text)
-        reactor_mock = self.mocker.patch(FakeReactor)
-
-        # This must necessarily happen in the following order.
-        self.mocker.order()
-
-        # This very informative message is printed out.
-        print_text_mock("Please wait... ", "")
-
-        time_mock = self.mocker.replace("time")
-        time_mock.sleep(ANY)
-        self.mocker.count(1)
-
-        # The register() method is called.  We fire the "registration-done"
-        # event after it's done, so that it cascades into a deferred callback.
-
-        def register_done():
-            service.reactor.fire("registration-done")
-
-        registration_mock.register()
-        self.mocker.call(register_done)
-
-        # The deferred callback finally prints out this message.
-        print_text_mock("System successfully registered.")
-
-        reactor_mock.stop()
-
-        # This is actually called after everything else since all deferreds
-        # are synchronous and callbacks will be executed immediately.
-        reactor_mock.run()
-
-        # Nothing else is printed!
-        print_text_mock(ANY)
-        self.mocker.count(0)
-
-        self.mocker.replay()
-
-        # DO IT!
-        return register(self.config, print_text, sys.exit,
-                        reactor=FakeReactor())
-
-    def test_register_failure(self):
-        """
-        When registration fails because of invalid credentials, a message will
-        be printed to the console and the program will exit.
-        """
-        service = self.broker_service
-
-        self.log_helper.ignore_errors(InvalidCredentialsError)
-        registration_mock = self.mocker.replace(service.registration)
-        print_text_mock = self.mocker.replace(print_text)
-        reactor_mock = self.mocker.patch(FakeReactor)
-
-        # This must necessarily happen in the following order.
-        self.mocker.order()
-
-        # This very informative message is printed out.
-        print_text_mock("Please wait... ", "")
-
-        time_mock = self.mocker.replace("time")
-        time_mock.sleep(ANY)
-        self.mocker.count(1)
-
-        # The register() method is called.  We fire the "registration-failed"
-        # event after it's done, so that it cascades into a deferred errback.
-
-        def register_done():
-            service.reactor.fire("registration-failed")
-
-        registration_mock.register()
-        self.mocker.call(register_done)
-
-        # The deferred errback finally prints out this message.
-        print_text_mock("Invalid account name or registration key.",
-                        error=True)
-
-        reactor_mock.stop()
-
-        # This is actually called after everything else since all deferreds
-        # are synchronous and callbacks will be executed immediately.
-        reactor_mock.run()
-
-        # Nothing else is printed!
-        print_text_mock(ANY)
-        self.mocker.count(0)
-
-        self.mocker.replay()
-
-        # DO IT!
-        exit = []
-        register(self.config, print_text, exit.append, reactor=FakeReactor())
-        self.assertEqual([2], exit)
-
-    def test_register_exchange_failure(self):
-        """
-        When registration fails because the server couldn't be contacted, a
-        message is printed and the program quits.
-        """
-        service = self.broker_service
-
-        registration_mock = self.mocker.replace(service.registration)
-        print_text_mock = self.mocker.replace(print_text)
-        reactor_mock = self.mocker.patch(FakeReactor)
-
-        # This must necessarily happen in the following order.
-        self.mocker.order()
-
-        # This very informative message is printed out.
-        print_text_mock("Please wait... ", "")
-
-        time_mock = self.mocker.replace("time")
-        time_mock.sleep(ANY)
-        self.mocker.count(1)
-
-        def register_done():
-            service.reactor.fire("exchange-failed")
-        registration_mock.register()
-        self.mocker.call(register_done)
-
-        # The deferred errback finally prints out this message.
-        print_text_mock("\nWe were unable to contact the server.\n"
-                        "Your internet connection may be down. "
-                        "The landscape client will continue to try and "
-                        "contact the server periodically.",
-                        error=True)
-
-        reactor_mock.stop()
-
-        # This is actually called after everything else since all deferreds
-        # are synchronous and callbacks will be executed immediately.
-        reactor_mock.run()
-
-        # Nothing else is printed!
-        print_text_mock(ANY)
-        self.mocker.count(0)
-
-        self.mocker.replay()
-
-        # DO IT!
-        exit = []
-        register(self.config, print_text, exit.append, reactor=FakeReactor())
-        self.assertEqual([2], exit)
-
-    def test_register_exchange_SSL_failure(self):
-        """
-        When registration fails because the server's SSL certificate could not
-        be validated, a message is printed and the program quits.
-        """
-        service = self.broker_service
-
-        registration_mock = self.mocker.replace(service.registration)
-        print_text_mock = self.mocker.replace(print_text)
-        reactor_mock = self.mocker.patch(FakeReactor)
-
-        # This must necessarily happen in the following order.
-        self.mocker.order()
-
-        # This very informative message is printed out.
-        print_text_mock("Please wait... ", "")
-
-        time_mock = self.mocker.replace("time")
-        time_mock.sleep(ANY)
-        self.mocker.count(1)
-
-        def register_done():
-            service.reactor.fire("exchange-failed", ssl_error=True)
-        registration_mock.register()
-        self.mocker.call(register_done)
-
-        # The deferred errback finally prints out this message.
-        print_text_mock("\nThe server's SSL information is incorrect, or "
-                        "fails signature verification!\n"
-                        "If the server is using a self-signed certificate, "
-                        "please ensure you supply it with the "
-                        "--ssl-public-key parameter.",
-                        error=True)
-
-        reactor_mock.stop()
-
-        # This is actually called after everything else since all deferreds
-        # are synchronous and callbacks will be executed immediately.
-        reactor_mock.run()
-
-        # Nothing else is printed!
-        print_text_mock(ANY)
-        self.mocker.count(0)
-
-        self.mocker.replay()
-
-        # DO IT!
-        exit = []
-        register(self.config, print_text, exit.append, reactor=FakeReactor())
-        self.assertEqual([2], exit)
-
-    def test_register_timeout_failure(self):
-        service = self.broker_service
-
-        registration_mock = self.mocker.replace(service.registration)
-        print_text_mock = self.mocker.replace(print_text)
-        reactor_mock = self.mocker.patch(FakeReactor)
-        remote_mock = self.mocker.patch(RemoteBroker)
-
-        protocol_mock = self.mocker.patch(MethodCallSender)
-        protocol_mock.timeout
-        self.mocker.result(0.1)
-        self.mocker.count(0, None)
-
-        # This must necessarily happen in the following order.
-        self.mocker.order()
-
-        # This very informative message is printed out.
-        print_text_mock("Please wait... ", "")
-
-        time_mock = self.mocker.replace("time")
-        time_mock.sleep(ANY)
-        self.mocker.count(1)
-
-        remote_mock.call_on_event(ANY)
-        self.mocker.result(succeed(None))
-
-        registration_mock.register()
-        self.mocker.passthrough()
-
-        # This is actually called after everything else since all deferreds
-        # are synchronous and callbacks will be executed immediately.
-        reactor_mock.run()
-
-        # Nothing else is printed!
-        print_text_mock(ANY)
-        self.mocker.count(0)
-
-        self.mocker.replay()
-
-        # DO IT!
-        fake_reactor = FakeReactor()
-        fake_reactor._reactor = Clock()
-        deferred = register(self.config, print_text, sys.exit,
-                            reactor=fake_reactor)
-        fake_reactor._reactor.advance(100)
-        return deferred
-
-    def test_register_bus_connection_failure(self):
-        """
-        If the socket can't be connected to, landscape-config will print an
-        explanatory message and exit cleanly.
-        """
-        # This will make the RemoteBrokerConnector.connect call fail
-        print_text_mock = self.mocker.replace(print_text)
-        time_mock = self.mocker.replace("time")
-        sys_mock = self.mocker.replace("sys")
-        reactor_mock = self.mocker.patch(LandscapeReactor)
-
-        connector_factory = self.mocker.replace(
-            "landscape.broker.amp.RemoteBrokerConnector", passthrough=False)
-        connector = connector_factory(ANY, ANY)
-        connector.connect(max_retries=0, quiet=True)
-        self.mocker.result(fail(ZeroDivisionError))
-
-        print_text_mock(ARGS)
-        time_mock.sleep(ANY)
-        reactor_mock.run()
-
-        print_text_mock(
-            CONTAINS("There was an error communicating with the "
-                     "Landscape client"),
-            error=True)
-        print_text_mock(CONTAINS("This machine will be registered"),
-                        error=True)
-
-        sys_mock.exit(2)
-        connector.disconnect()
-        reactor_mock.stop()
-
-        self.mocker.replay()
-
-        config = self.get_config(["-a", "accountname", "--silent"])
-        return register(config, print_text, sys.exit, max_retries=0)
-
-    def test_register_bus_connection_failure_ok_no_register(self):
-        """
-        Exit code 0 will be returned if we can't contact Landscape via DBus and
-        --ok-no-register was passed.
-        """
-        print_text_mock = self.mocker.replace(print_text)
-        time_mock = self.mocker.replace("time")
-        reactor_mock = self.mocker.patch(LandscapeReactor)
-
-        print_text_mock(ARGS)
-        time_mock.sleep(ANY)
-        reactor_mock.run()
-        reactor_mock.stop()
-
-        print_text_mock(
-            CONTAINS("There was an error communicating with the "
-                     "Landscape client"),
-            error=True)
-        print_text_mock(CONTAINS("This machine will be registered"),
-                        error=True)
-
-        self.mocker.replay()
-
-        config = self.get_config(
-            ["-a", "accountname", "--silent", "--ok-no-register"])
-        return self.assertSuccess(register(config, print_text, sys.exit,
-                                           max_retries=0))
-
-
-class RegisterFunctionRetryTest(LandscapeConfigurationTest):
-
-    helpers = [BrokerServiceHelper]
-
-    def setUp(self):
-        super(RegisterFunctionRetryTest, self).setUp()
-        self.config = LandscapeSetupConfiguration()
-        self.config.load(["-c", self.config_filename])
-
-    def test_register_with_retry_parameters(self):
-        """
-        Retry parameters are passed to the L{connect} method of the connector.
-        """
-        print_text_mock = self.mocker.replace(print_text)
-        time_mock = self.mocker.replace("time")
-        sys_mock = self.mocker.replace("sys")
-        reactor_mock = self.mocker.patch(LandscapeReactor)
-
-        connector_factory = self.mocker.replace(
-            "landscape.broker.amp.RemoteBrokerConnector", passthrough=False)
-        connector = connector_factory(ANY, ANY)
-        connector.connect(quiet=True, max_retries=12)
-
-        self.mocker.result(succeed(None))
-
-        print_text_mock(ARGS)
-        time_mock.sleep(ANY)
-        reactor_mock.run()
-
-        print_text_mock(
-            CONTAINS("There was an error communicating with the "
-                     "Landscape client"),
-            error=True)
-        print_text_mock(CONTAINS("This machine will be registered"),
-                        error=True)
-
-        sys_mock.exit(2)
-        connector.disconnect()
-        reactor_mock.stop()
-
-        self.mocker.replay()
-
-        config = self.get_config(["-a", "accountname", "--silent"])
-        return register(config, print_text, sys.exit, max_retries=12)
-
-    def test_register_with_default_retry_parameters(self):
-        """
-        max_retries has reasonable default behavior - retry 14 times which
-        will result in a wait of about 60 seconds, until the broker has time
-        to start on heavily loaded systems.
-
-        initialDelay = 0.05
-        factor =  1.62
-        maxDelay = 30
-        max_retries = 14
-
-        0.05 * (1 - 1.62 ** 14) / (1 - 1.62) = 69 seconds
-        """
-        print_text_mock = self.mocker.replace(print_text)
-        time_mock = self.mocker.replace("time")
-        sys_mock = self.mocker.replace("sys")
-        reactor_mock = self.mocker.patch(LandscapeReactor)
-
-        connector_factory = self.mocker.replace(
-            "landscape.broker.amp.RemoteBrokerConnector", passthrough=False)
-        connector = connector_factory(ANY, ANY)
-        connector.connect(quiet=True, max_retries=14)
-
-        self.mocker.result(succeed(None))
-
-        print_text_mock(ARGS)
-        time_mock.sleep(ANY)
-        reactor_mock.run()
-
-        print_text_mock(
-            CONTAINS("There was an error communicating with the "
-                     "Landscape client"),
-            error=True)
-        print_text_mock(CONTAINS("This machine will be registered"),
-                        error=True)
-
-        sys_mock.exit(2)
-        connector.disconnect()
-        reactor_mock.stop()
-
-        self.mocker.replay()
-
-        config = self.get_config(["-a", "accountname", "--silent"])
-        return register(config, print_text, sys.exit)
-
-
-class RegisterFunctionNoServiceTest(LandscapeTest):
-
-    def test_register_unknown_error(self):
-        """
-        When registration fails because of an unknown error, a message is
-        printed and the program exits.
-        """
-        configuration = LandscapeSetupConfiguration()
-
-        # We'll just mock the remote here to have it raise an exception.
-        connector_factory = self.mocker.replace(
-            "landscape.broker.amp.RemoteBrokerConnector", passthrough=False)
-        remote_broker = self.mocker.mock()
-
-        print_text_mock = self.mocker.replace(print_text)
-        reactor_mock = self.mocker.patch(FakeReactor)
-
-        # This is unordered. It's just way too much of a pain.
-        print_text_mock("Please wait... ", "")
-        time_mock = self.mocker.replace("time")
-        time_mock.sleep(ANY)
-        self.mocker.count(1)
-
-        # SNORE
-        connector = connector_factory(ANY, configuration)
-        connector.connect(max_retries=0, quiet=True)
-        self.mocker.result(succeed(remote_broker))
-        remote_broker.call_on_event(ANY)
-        self.mocker.result(succeed(None))
-
-        # here it is!
-        remote_broker.register()
-        self.mocker.result(fail(ZeroDivisionError))
-
-        print_text_mock(ANY, error=True)
-
-        def check_logged_failure(text, error):
-            self.assertTrue("ZeroDivisionError" in text)
-        self.mocker.call(check_logged_failure)
-        print_text_mock("Unknown error occurred.", error=True)
-
-        # WHOAH DUDE. This waits for callLater(0, reactor.stop).
-        connector.disconnect()
-        reactor_mock.stop()
-        reactor_mock.run()
-
-        self.mocker.replay()
-
-        exit = []
-        register(configuration, print_text, exit.append, max_retries=0,
-                 reactor=FakeReactor())
-        self.assertEqual(exit, [2])
+    def test_register(self):
+        """Is the async machinery wired up properly?"""
+
+        class FauxFailure(object):
+            def getTraceback(self):
+                return 'traceback'
+
+        class FauxReactor(object):
+            def run(self):
+                self.was_run = True
+
+            def stop(self, *args):
+                self.was_stopped = True
+
+        reactor = FauxReactor()
+        connector = FauxConnector(reactor, self.config)
+
+        def connector_factory(reactor, config):
+            return connector
+
+        # We pre-seed a success because no actual result will be generated.
+        register(self.config, reactor, connector_factory, max_retries=99,
+            results=['success'])
+        self.assertTrue(reactor.was_run)
+        # Only a single callback is registered, it does the real work when a
+        # connection is established.
+        self.assertTrue(1, len(connector.connection.callbacks))
+        self.assertEqual(
+            'got_connection',
+            connector.connection.callbacks[0].func.__name__)
+        # Should something go wrong, there is an error handler registered.
+        self.assertTrue(1, len(connector.connection.errbacks))
+        self.assertEqual(
+            'got_error',
+            connector.connection.errbacks[0].__name__)
+        # We ask for retries because networks aren't reliable.
+        self.assertEqual(99, connector.max_retries)
+
+    def test_got_connection(self):
+        """got_connection() adds deferreds and callbacks."""
+
+        def faux_got_connection(add_result, remote, connector, reactor):
+            pass
+
+        class FauxRemote(object):
+            handlers = None
+            deferred = None
+
+            def call_on_event(self, handlers):
+                assert not self.handlers, "Called twice"
+                self.handlers = handlers
+                self.call_on_event_deferred = FauxCallOnEventDeferred()
+                return self.call_on_event_deferred
+
+            def register(self):
+                assert not self.deferred, "Called twice"
+                self.register_deferred = FauxRegisterDeferred()
+                return self.register_deferred
+
+        class FauxCallOnEventDeferred(object):
+            def __init__(self):
+                self.callbacks = []
+                self.errbacks = []
+
+            def addCallbacks(self, *funcs, **kws):
+                self.callbacks.extend(funcs)
+
+        class FauxRegisterDeferred(object):
+            def __init__(self):
+                self.callbacks = []
+                self.errbacks = []
+
+            def addCallback(self, func):
+                assert func.__name__ == "got_connection", "Wrong callback."
+                self.callbacks.append(faux_got_connection)
+                self.gather_results_deferred = GatherResultsDeferred()
+                return self.gather_results_deferred
+
+            def addCallbacks(self, *funcs, **kws):
+                self.callbacks.extend(funcs)
+
+            def addErrback(self, func, *args, **kws):
+                self.errbacks.append(func)
+                return self
+
+        class GatherResultsDeferred(object):
+            def __init__(self):
+                self.callbacks = []
+                self.errbacks = []
+
+            def addCallbacks(self, *funcs, **kws):
+                self.callbacks.extend(funcs)
+
+        faux_connector = FauxConnector(self.reactor, self.config)
+
+        status_results = []
+        faux_remote = FauxRemote()
+        results = got_connection(
+            status_results.append, faux_connector, self.reactor, faux_remote)
+        # We set up two deferreds, one for the RPC call and one for event
+        # handlers.
+        self.assertEqual(2, len(results.resultList))
+        # Handlers are registered for the events we are interested in.
+        self.assertEqual(
+            ['registration-failed', 'exchange-failed', 'registration-done'],
+            faux_remote.handlers.keys())
+        self.assertEqual(
+            ['failure', 'exchange_failure', 'success'],
+            [handler.func.__name__
+                for handler in faux_remote.handlers.values()])
+        # We include a single error handler to react to exchange errors.
+        self.assertTrue(1, len(faux_remote.register_deferred.errbacks))
+        self.assertEqual(
+            'handle_registration_errors',
+            faux_remote.register_deferred.errbacks[0].__name__)
+
+    def test_register_happy_path(self):
+        """A successful result provokes no exceptions."""
+        def faux_got_connection(add_result, remote, connector, reactor):
+            add_result('success')
+        self.reactor.call_later(1, self.reactor.stop)
+        self.assertEqual(
+            "success",
+            register(self.config, reactor=self.reactor,
+                got_connection=faux_got_connection))
 
 
 class SSLCertificateDataTest(LandscapeConfigurationTest):
@@ -2159,3 +2121,41 @@
         self.assertEqual(key_filename,
                          store_public_key_data(config, "123456789"))
         self.assertEqual("123456789", open(key_filename, "r").read())
+
+
+class ReportRegistrationOutcomeTest(unittest.TestCase):
+
+    def setUp(self):
+        self.result = []
+        self.output = []
+
+    def record_result(self, result, file=sys.stdout):
+        self.result.append(result)
+        self.output.append(file.name)
+
+    def test_success_case(self):
+        report_registration_outcome("success", print=self.record_result)
+        self.assertIn("System successfully registered.", self.result)
+        self.assertIn(sys.stdout.name, self.output)
+
+    def test_failure_case(self):
+        report_registration_outcome("failure", print=self.record_result)
+        self.assertIn("Invalid account name or registration key.", self.result)
+        self.assertIn(sys.stderr.name, self.output)
+
+    def test_ssl_error_case(self):
+        report_registration_outcome("ssl-error", print=self.record_result)
+        self.assertIn("\nThe server's SSL information is incorrect, or fails "
+              "signature verification!\n"
+              "If the server is using a self-signed certificate, "
+              "please ensure you supply it with the --ssl-public-key "
+              "parameter.", self.result)
+        self.assertIn(sys.stderr.name, self.output)
+
+    def test_non_ssl_error_case(self):
+        report_registration_outcome("non-ssl-error", print=self.record_result)
+        self.assertIn("\nWe were unable to contact the server.\n"
+              "Your internet connection may be down. "
+              "The landscape client will continue to try and contact "
+              "the server periodically.", self.result)
+        self.assertIn(sys.stderr.name, self.output)