Merge lp:~tealeg/landscape-client/warn-on-unicode into lp:~landscape/landscape-client/trunk

[Geoff Teale]

Fixes bug #956612.

 - Extends the InfoBar implementation from the sanitise-hostname branch to allow for more than one error to be displayed in parallel.
 - Adds a check for non-ASCII input in Gtk.Entry fields.

[Mike Milner (milner)]

Looks good, but there are two aspects that should be looked at:

[1]
35 - def is_valid_host_name(self, host_name):
36 - return HOSTNAME_REGEXP.match(host_name) is not None
37 + def is_valid_host_name(self, entry):
38 + host_name = self.sanitise_host_name(entry.get_text())

You're mixing concerns here. The is_valid_host_name (and is_ascii) functions now have side effects on the dialog. It would be better to keep the is_valid_host_name separate from the dialog-specific code. This makes the functions easy to test without worrying about the dialog side effects.

[2]
165 + def test_input_is_ascii(self):
166 + """
167 + Test that we can correctly identify non-ASCII input in a L{Gtk.Entry}.
168 + """
169 + dialog = ClientSettingsDialog(self.controller)
170 + self.run_gtk_eventloop()
171 + dialog.use_type_combobox.set_active(1)
172 + self.run_gtk_eventloop()
173 + dialog.hosted_account_name_entry.set_text("Toodleoo")
174 + self.assertTrue(dialog.is_ascii(dialog.hosted_account_name_entry))
175 + self.assertEqual(0, len(dialog._errored_entries))
176 + dialog.hosted_account_name_entry.set_text(u"T\xc3dle\xc4")
177 + self.assertFalse(dialog.is_ascii(dialog.hosted_account_name_entry))
178 + self.assertEqual(1, len(dialog._errored_entries))

This test should be broken into two. Each test should have the minimum amount of code to verify one expected behaviour. Generally, tests should have the following structure: SETUP, RUN-CODE, ASSERT. If there are two groups of asserts there should probably be two tests. Same goes for test_dismiss_validation_errors.

review: Needs Fixing

[lp:~tealeg/landscape-client/warn-on-unicode]

552. By Geoff Teale on 2012-03-27

Restructure to avoid side effects and structure tests better (from milners review)

553. By Geoff Teale on 2012-03-28

merged forwards from trunk.

554. By Geoff Teale on 2012-03-28

lint

[Geoff Teale (tealeg)]

> Looks good, but there are two aspects that should be looked at:
>
> [1]
> 35 - def is_valid_host_name(self, host_name):
> 36 - return HOSTNAME_REGEXP.match(host_name) is not None
> 37 + def is_valid_host_name(self, entry):
> 38 + host_name = self.sanitise_host_name(entry.get_text())
>
> You're mixing concerns here. The is_valid_host_name (and is_ascii) functions
> now have side effects on the dialog. It would be better to keep the
> is_valid_host_name separate from the dialog-specific code. This makes the
> functions easy to test without worrying about the dialog side effects.

Very good point indeed - for some reason I couldn't see a sensible way around this the other day, but today it seems obvious. Anyhow, done.

>
>
> [2]
> 165 + def test_input_is_ascii(self):
> 166 + """
> 167 + Test that we can correctly identify non-ASCII input in a
> L{Gtk.Entry}.
> 168 + """
> 169 + dialog = ClientSettingsDialog(self.controller)
> 170 + self.run_gtk_eventloop()
> 171 + dialog.use_type_combobox.set_active(1)
> 172 + self.run_gtk_eventloop()
> 173 + dialog.hosted_account_name_entry.set_text("Toodleoo")
> 174 +
> self.assertTrue(dialog.is_ascii(dialog.hosted_account_name_entry))
> 175 + self.assertEqual(0, len(dialog._errored_entries))
> 176 + dialog.hosted_account_name_entry.set_text(u"T\xc3dle\xc4")
> 177 +
> self.assertFalse(dialog.is_ascii(dialog.hosted_account_name_entry))
> 178 + self.assertEqual(1, len(dialog._errored_entries))
>
> This test should be broken into two. Each test should have the minimum amount
> of code to verify one expected behaviour. Generally, tests should have the
> following structure: SETUP, RUN-CODE, ASSERT. If there are two groups of
> asserts there should probably be two tests. Same goes for
> test_dismiss_validation_errors.

I've done quite an extensive clean up here now. With specific regard to test_dismiss_validation_errors - I have split it into local and hosted tests, but I retain assertions halfway through those tests - that stems from my personal preference to assert that a condition exists in my test if I am testing a later action that changes that state. I can see that some people might think that pollutes the meaning of the test, but I find it ensures the validity of the result. Please advise if you find this distasteful.

[lp:~tealeg/landscape-client/warn-on-unicode]

555. By Geoff Teale on 2012-03-28

Split test as per milner's review request.

[Mike Milner (milner)]

> I've done quite an extensive clean up here now. With specific regard to
> test_dismiss_validation_errors - I have split it into local and hosted tests,
> but I retain assertions halfway through those tests - that stems from my
> personal preference to assert that a condition exists in my test if I am
> testing a later action that changes that state. I can see that some people
> might think that pollutes the meaning of the test, but I find it ensures the
> validity of the result. Please advise if you find this distasteful.

Yeah checking preconditions makes sense, it was mainly about keeping the tests focused. If you're ever bored check out the S3 tests in boto for a great example of a monolithic super mega test and you'll see the bad extreme of this :)

Looks great! I especially like the Bob the Builder references ;) +1

review: Approve

[Geoff Teale (tealeg)]

>
> Yeah checking preconditions makes sense, it was mainly about keeping the tests
> focused. If you're ever bored check out the S3 tests in boto for a great
> example of a monolithic super mega test and you'll see the bad extreme of this
> :)

I'll save that for a slow day ;-)

> Looks great! I especially like the Bob the Builder references ;) +1

Parenthood is pervasive.

Cheers for the review!

[Alberto Donato (ack)]

Great! +1

#1:
+ else:
+ self.indicate_error_on_entry(self.local_landscape_host_entry)
+ if not host_name_ok:
+ self._validation_errors.add(self.INVALID_HOST_NAME)
+ if not ascii_ok:
+ self._validation_errors.add(self.UNICODE_IN_ENTRY)
+ return False

return False should be up one level of indentation (under the 'else' case)

review: Approve

[lp:~tealeg/landscape-client/warn-on-unicode]

556. By Geoff Teale on 2012-03-28

Correctly indent return. From ack's review.

=== modified file 'landscape/ui/view/configuration.py'
--- landscape/ui/view/configuration.py	2012-03-28 08:32:37 +0000
+++ landscape/ui/view/configuration.py	2012-03-28 14:18:17 +0000
@@ -9,8 +9,34 @@
     CANONICAL_MANAGED, LOCAL_MANAGED, NOT_MANAGED)
 
 # Note, I think this may not be fully compliant with the changes in RFC 1123
-HOSTNAME_REGEXP = re.compile("^(([a-zA-Z][a-zA-Z0-9\-]*)?[a-zA-Z0-9][\.]?)*"
-                             "(([A-Za-z][A-Za-z0-9\-]*)?[A-Za-z0-9])$")
+HOST_NAME_REGEXP = re.compile("^(([a-zA-Z][a-zA-Z0-9\-]*)?[a-zA-Z0-9][\.]?)*"
+                              "(([A-Za-z][A-Za-z0-9\-]*)?[A-Za-z0-9])$")
+
+
+def sanitise_host_name(host_name):
+    """
+    Do some minimal host name sanitation.
+    """
+    return host_name.strip()
+
+
+def is_valid_host_name(host_name):
+    """
+    Check that the provided host name complies with L{HOST_NAME_REGEXP} and is
+    therefor valid.
+    """
+    return HOST_NAME_REGEXP.match(host_name) is not None
+
+
+def is_ascii(text):
+    """
+    Test that the provided string contains only characters from the ASCII set.
+    """
+    try:
+        text.decode("ascii")
+        return True
+    except UnicodeDecodeError:
+        return False
 
 
 class ClientSettingsDialog(Gtk.Dialog):
@@ -21,6 +47,8 @@
     """
 
     GLADE_FILE = "landscape-client-settings.glade"
+    INVALID_HOST_NAME = 0
+    UNICODE_IN_ENTRY = 1
 
     def __init__(self, controller):
         super(ClientSettingsDialog, self).__init__(
@@ -29,37 +57,67 @@
         self.set_default_icon_name("preferences-management-service")
         self.set_resizable(False)
         self._initialised = False
+        self._validation_errors = set()
+        self._errored_entries = []
         self.controller = controller
         self.setup_ui()
         self.load_data()
         # One extra revert to reset after loading data
         self.controller.revert()
 
-    def sanitise_host_name(self, host_name):
-        """
-        Do some minimal input sanitation.
-        """
-        return host_name.strip()
-
-    def is_valid_host_name(self, host_name):
-        return HOSTNAME_REGEXP.match(host_name) is not None
+    def indicate_error_on_entry(self, entry):
+        """
+        Show a warning icon on a L{Gtk.Entry} to indicate some associated
+        error.
+        """
+        entry.set_icon_from_stock(
+            Gtk.EntryIconPosition.PRIMARY, Gtk.STOCK_DIALOG_WARNING)
+        self._errored_entries.append(entry)
+
+    def check_local_landscape_host_name_entry(self):
+        host_name = sanitise_host_name(
+            self.local_landscape_host_entry.get_text())
+        ascii_ok = is_ascii(host_name)
+        host_name_ok = is_valid_host_name(host_name)
+        if ascii_ok and host_name_ok:
+            self.local_landscape_host_entry.set_text(host_name)
+            return True
+        else:
+            self.indicate_error_on_entry(self.local_landscape_host_entry)
+            if not host_name_ok:
+                self._validation_errors.add(self.INVALID_HOST_NAME)
+            if not ascii_ok:
+                self._validation_errors.add(self.UNICODE_IN_ENTRY)
+            return False
+
+    def check_entry(self, entry):
+        """
+        Check that the text content of a L{Gtk.Entry} is acceptable.
+        """
+        if is_ascii(entry.get_text()):
+            return True
+        else:
+            self.indicate_error_on_entry(entry)
+            self._validation_errors.add(self.UNICODE_IN_ENTRY)
+            return False
 
     def validity_check(self):
-        if self.use_type_combobox.get_active() < 2:
+        self._validation_errors = set()
+        if self._info_bar_container.get_visible():
+            self.dismiss_infobar(None)
+        active_iter = self.liststore.get_iter(
+            self.use_type_combobox.get_active())
+        [management_type] = self.liststore.get(active_iter, 0)
+        if management_type == NOT_MANAGED:
             return True
+        elif management_type == CANONICAL_MANAGED:
+            account_name_ok = self.check_entry(self.hosted_account_name_entry)
+            password_ok = self.check_entry(self.hosted_password_entry)
+            return account_name_ok and password_ok
         else:
-            host_name = self.sanitise_host_name(
-                self.local_landscape_host_entry.get_text())
-            if self.is_valid_host_name(host_name):
-                self.local_landscape_host_entry.set_text(host_name)
-                return True
-            else:
-                self.info_message.set_text(self.INVALID_HOSTNAME_MESSAGE)
-                self.local_landscape_host_entry.set_icon_from_stock(
-                    Gtk.EntryIconPosition.PRIMARY,
-                    Gtk.STOCK_DIALOG_WARNING)
-                self._info_bar_container.show()
-                return False
+            host_name_ok = self.check_local_landscape_host_name_entry()
+            password_ok = self.check_entry(self.local_password_entry)
+            return host_name_ok and password_ok
 
     @property
     def NO_SERVICE_TEXT(self):
@@ -82,9 +140,13 @@
         return _("Disable")
 
     @property
-    def INVALID_HOSTNAME_MESSAGE(self):
+    def INVALID_HOST_NAME_MESSAGE(self):
         return _("Invalid host name.")
 
+    @property
+    def UNICODE_IN_ENTRY_MESSAGE(self):
+        return _("Only ASCII characters are allowed.")
+
     def _set_use_type_combobox_from_controller(self):
         """
         Load the persisted L{management_type} from the controller and set the
@@ -177,6 +239,14 @@
     def register_response(self, widget):
         if self.validity_check():
             self.response(Gtk.ResponseType.OK)
+        else:
+            error_text = []
+            if self.UNICODE_IN_ENTRY in self._validation_errors:
+                error_text.append(self.UNICODE_IN_ENTRY_MESSAGE)
+            if self.INVALID_HOST_NAME in self._validation_errors:
+                error_text.append(self.INVALID_HOST_NAME_MESSAGE)
+            self.info_message.set_text("\n".join(error_text))
+            self._info_bar_container.show()
 
     def set_button_text(self, management_type):
         [alignment] = self.register_button.get_children()
@@ -203,8 +273,9 @@
 
     def dismiss_infobar(self, widget):
         self._info_bar_container.hide()
-        self.local_landscape_host_entry.set_icon_from_stock(
-            Gtk.EntryIconPosition.PRIMARY, None)
+        for entry in self._errored_entries:
+            entry.set_icon_from_stock(Gtk.EntryIconPosition.PRIMARY, None)
+        self._errored_entries = []
 
     def setup_info_bar(self):
         labels_size_group = self._builder.get_object("labels-sizegroup")

=== modified file 'landscape/ui/view/tests/test_configuration.py'
--- landscape/ui/view/tests/test_configuration.py	2012-03-28 08:18:47 +0000
+++ landscape/ui/view/tests/test_configuration.py	2012-03-28 14:18:17 +0000
@@ -6,7 +6,8 @@
 
 if got_gobject_introspection:
     from gi.repository import Gtk
-    from landscape.ui.view.configuration import ClientSettingsDialog
+    from landscape.ui.view.configuration import (
+        ClientSettingsDialog, sanitise_host_name, is_valid_host_name)
     from landscape.ui.controller.configuration import ConfigController
     import landscape.ui.model.configuration.state
     from landscape.ui.model.configuration.state import (
@@ -16,6 +17,47 @@
 from landscape.tests.helpers import LandscapeTest
 
 
+class ViewFunctionsTest(LandscapeTest):
+
+    def test_sanitise_host_name(self):
+        """
+        Test UI level host_name sanitation.
+        """
+        self.assertEqual("foo.bar", sanitise_host_name(" foo.bar"))
+        self.assertEqual("foo.bar", sanitise_host_name("foo.bar "))
+        self.assertEqual("foo.bar", sanitise_host_name(" foo.bar "))
+
+    def test_is_valid_host_name_ok(self):
+        """
+        Test that valid host names cause L{is_valid_host_name} to return
+        L{True}.
+        """
+        self.assertTrue(is_valid_host_name("a"))
+        self.assertTrue(is_valid_host_name("a.b"))
+        self.assertTrue(is_valid_host_name("a.b.c"))
+        self.assertTrue(is_valid_host_name("stop-squark"))
+        self.assertTrue(is_valid_host_name("stop-squark.teale.DE"))
+        self.assertTrue(is_valid_host_name("a2.b3.c4"))
+
+    def test_is_valid_host_name_bad(self):
+        """
+        Test that invalid host names cause L{is_valid_host_name} to return
+        L{False}.
+        """
+        self.assertFalse(is_valid_host_name(".a"))
+        self.assertFalse(is_valid_host_name("a."))
+        self.assertFalse(is_valid_host_name("a b"))
+        self.assertFalse(is_valid_host_name("a .b"))
+        self.assertFalse(is_valid_host_name("a. b"))
+
+    def test_is_valid_host_name_unicode(self):
+        """
+        Test that host names containing Unicode cause L{is_valid_host_name} to
+        return L{False}.
+        """
+        self.assertFalse(is_valid_host_name(u"\xc3a"))
+
+
 class ConfigurationViewTest(LandscapeTest):
 
     helpers = [ConfigurationProxyHelper]
@@ -210,58 +252,287 @@
         self.assertEqual("foo", dialog.hosted_account_name_entry.get_text())
         self.assertEqual("bar", dialog.hosted_password_entry.get_text())
 
-    def test_sanitise_host_name(self):
-        """
-        Test that L{ClientSettingsDialog.sanitise_host_name} removes both
-        leading and trailing white space from a host name.
-        """
-        dialog = ClientSettingsDialog(self.controller)
-        self.assertEquals("foo.bar", dialog.sanitise_host_name(" foo.bar"))
-        self.assertEquals("foo.bar", dialog.sanitise_host_name("foo.bar "))
-        self.assertEquals("foo.bar", dialog.sanitise_host_name(" foo.bar "))
-
-    def test_is_valid_host_name(self):
-        """
-        Test that L{is_valid_host_name} checks host names correctly.
-        """
-        dialog = ClientSettingsDialog(self.controller)
-        self.assertTrue(dialog.is_valid_host_name("foo.bar"))
-        self.assertFalse(dialog.is_valid_host_name("foo bar"))
-        self.assertFalse(dialog.is_valid_host_name("f\xc3.bar"))
-
-    def test_validity_check(self):
-        """
-        Test that the L{validity_check} returns True for valid input and False
-        for invalid input.
-        """
-        dialog = ClientSettingsDialog(self.controller)
-        self.run_gtk_eventloop()
-
-        # Checking disable should always return True
+    def test_check_local_landscape_host_name_entry_ok(self):
+        """
+        Test that L{check_local_landscape_host_name_entry} returns L{True} when
+        the input is a valid host name.
+        """
+        dialog = ClientSettingsDialog(self.controller)
+        dialog.use_type_combobox.set_active(2)
+        dialog.local_landscape_host_entry.set_text("foo.bar")
+        self.assertTrue(dialog.check_local_landscape_host_name_entry())
+
+    def test_check_local_landscape_host_name_entry_ok_not_recorded(self):
+        """
+        Test that L{check_local_landscape_host_name_entry} does not add the
+        entry to L{ClientSettingsDialog._errored_entries} when the input is a
+        valid host name.
+        """
+        dialog = ClientSettingsDialog(self.controller)
+        dialog.use_type_combobox.set_active(2)
+        dialog.local_landscape_host_entry.set_text("foo.bar")
+        dialog.check_local_landscape_host_name_entry()
+        self.assertEqual(0, len(dialog._errored_entries))
+
+    def test_check_local_landscape_host_name_entry_bad_host_name(self):
+        """
+        Test that L{check_local_landscape_host_name_entry} returns L{False}
+        when the input is not a valid host name.
+        """
+        dialog = ClientSettingsDialog(self.controller)
+        dialog.use_type_combobox.set_active(2)
+        dialog.local_landscape_host_entry.set_text("foo bar")
+        self.assertFalse(dialog.check_local_landscape_host_name_entry())
+
+    def test_check_local_landscape_host_name_entry_bad_recorded(self):
+        """
+        Test that L{check_local_landscape_host_name_entry} does add the
+        entry to L{ClientSettingsDialog._errored_entries} when the input is not
+        a valid host name.
+        """
+        dialog = ClientSettingsDialog(self.controller)
+        dialog.use_type_combobox.set_active(2)
+        dialog.local_landscape_host_entry.set_text("foo bar")
+        dialog.check_local_landscape_host_name_entry()
+        self.assertEqual(1, len(dialog._errored_entries))
+
+    def test_check_local_landscape_host_name_entry_bad_error_type(self):
+        """
+        Test that L{check_local_landscape_host_name_entry} adds the correct
+        error type to L{ClientSettingsDialog._validation_errors} when the input
+        is not a valid host name.
+        """
+        dialog = ClientSettingsDialog(self.controller)
+        dialog.use_type_combobox.set_active(2)
+        dialog.local_landscape_host_entry.set_text("foo bar")
+        dialog.check_local_landscape_host_name_entry()
+        self.assertEqual(set([dialog.INVALID_HOST_NAME]),
+                         dialog._validation_errors)
+
+    def test_check_local_landscape_host_name_entry_unicode_in_host_name(self):
+        """
+        Test that L{check_local_landscape_host_name_entry} returns L{False}
+        when the input contains Unicode.
+        """
+        dialog = ClientSettingsDialog(self.controller)
+        dialog.use_type_combobox.set_active(2)
+        dialog.local_landscape_host_entry.set_text(u"f\xc3.bar")
+        self.assertFalse(dialog.check_local_landscape_host_name_entry())
+
+    def test_check_local_landscape_host_name_entry_unicode_recorded(self):
+        """
+        Test that L{check_local_landscape_host_name_entry} does add the
+        entry to L{ClientSettingsDialog._errored_entries} when the input
+        contains Unicode.
+        """
+        dialog = ClientSettingsDialog(self.controller)
+        dialog.use_type_combobox.set_active(2)
+        dialog.local_landscape_host_entry.set_text(u"f\xc3.bar")
+        dialog.check_local_landscape_host_name_entry()
+        self.assertEqual(1, len(dialog._errored_entries))
+
+    def test_check_local_landscape_host_name_entry_unicode_error_type(self):
+        """
+        Test that L{check_local_landscape_host_name_entry} adds the correct
+        error type to L{ClientSettingsDialog._validation_errors} when the input
+        contains Unicode.
+        """
+        dialog = ClientSettingsDialog(self.controller)
+        dialog.use_type_combobox.set_active(2)
+        dialog.local_landscape_host_entry.set_text(u"f\xc3.bar")
+        dialog.check_local_landscape_host_name_entry()
+        self.assertEqual(
+            set([dialog.INVALID_HOST_NAME, dialog.UNICODE_IN_ENTRY]),
+            dialog._validation_errors)
+
+    def test_check_entry_ok(self):
+        """
+        Test that we return L{True} when the text of a L{Gtk.Entry} is valid
+        input.
+        """
+        dialog = ClientSettingsDialog(self.controller)
+        self.run_gtk_eventloop()
+        dialog.use_type_combobox.set_active(1)
+        self.run_gtk_eventloop()
+        dialog.hosted_account_name_entry.set_text("Toodleoo")
+        self.assertTrue(dialog.check_entry(dialog.hosted_account_name_entry))
+
+    def test_check_entry_doesnt_record_entry_when_ok(self):
+        """
+        Test that, when the text of a L{Gtk.Entry} is valid nothing is added to
+        L{ClientSettingsDialog._errored_entries}.
+        """
+        dialog = ClientSettingsDialog(self.controller)
+        self.run_gtk_eventloop()
+        dialog.use_type_combobox.set_active(1)
+        self.run_gtk_eventloop()
+        dialog.hosted_account_name_entry.set_text("Toodleoo")
+        dialog.check_entry(dialog.hosted_account_name_entry)
+        self.assertEqual(0, len(dialog._errored_entries))
+
+    def test_check_entry_non_ascii(self):
+        """
+        Test that we return L{False} when the text of a L{Gtk.Entry} contains
+        Unicode input.
+        """
+        dialog = ClientSettingsDialog(self.controller)
+        self.run_gtk_eventloop()
+        dialog.use_type_combobox.set_active(1)
+        self.run_gtk_eventloop()
+        dialog.hosted_account_name_entry.set_text(u"T\xc3dle\xc4")
+        self.assertFalse(dialog.check_entry(dialog.hosted_account_name_entry))
+
+    def test_check_entry_records_entry_when_non_ascii(self):
+        """
+        Test that, when the text of a L{Gtk.Entry} contains Unicode it is
+        added to L{ClientSettingsDialog._errored_entries}.
+        """
+        dialog = ClientSettingsDialog(self.controller)
+        self.run_gtk_eventloop()
+        dialog.use_type_combobox.set_active(1)
+        self.run_gtk_eventloop()
+        dialog.hosted_account_name_entry.set_text(u"T\xc3dle\xc4")
+        dialog.check_entry(dialog.hosted_account_name_entry)
+        self.assertEqual(1, len(dialog._errored_entries))
+
+    def test_dismiss_validation_errors_local(self):
+        """
+        Test that dismissing the validation errors tidies up indicators that
+        have been set against local settings.
+        """
+        dialog = ClientSettingsDialog(self.controller)
+        self.run_gtk_eventloop()
+        dialog.use_type_combobox.set_active(1)
+        self.run_gtk_eventloop()
+        dialog.hosted_account_name_entry.set_text(u"T\xc3dle\xc4")
+        dialog.hosted_password_entry.set_text(u"T\xc3dle\xc4")
+        self.run_gtk_eventloop()
+        dialog.validity_check()
+        self.run_gtk_eventloop()
+        self.assertEqual(2, len(dialog._errored_entries))
+        [entry1, entry2] = dialog._errored_entries
+        self.assertEqual(Gtk.STOCK_DIALOG_WARNING,
+                         entry1.get_icon_stock(Gtk.EntryIconPosition.PRIMARY))
+        self.assertEqual(Gtk.STOCK_DIALOG_WARNING,
+                         entry2.get_icon_stock(Gtk.EntryIconPosition.PRIMARY))
+        dialog.dismiss_infobar(None)
+        self.run_gtk_eventloop()
+        self.assertEqual(0, len(dialog._errored_entries))
+        self.assertNotEqual(
+            Gtk.STOCK_DIALOG_WARNING,
+            entry1.get_icon_stock(Gtk.EntryIconPosition.PRIMARY))
+        self.assertNotEqual(
+            Gtk.STOCK_DIALOG_WARNING,
+            entry2.get_icon_stock(Gtk.EntryIconPosition.PRIMARY))
+
+    def test_dismiss_validation_errors_hosted(self):
+        """
+        Test that dismissing the validation errors tidies up indicators that
+        have been set against hosted fields.
+        """
+        dialog = ClientSettingsDialog(self.controller)
+        self.run_gtk_eventloop()
+        dialog.use_type_combobox.set_active(2)
+        self.run_gtk_eventloop()
+        dialog.local_landscape_host_entry.set_text("dodgy as hell")
+        self.run_gtk_eventloop()
+        dialog.validity_check()
+        self.run_gtk_eventloop()
+        self.assertEqual(1, len(dialog._errored_entries))
+        [entry1] = dialog._errored_entries
+        self.assertEqual(Gtk.STOCK_DIALOG_WARNING,
+                         entry1.get_icon_stock(Gtk.EntryIconPosition.PRIMARY))
+        dialog.dismiss_infobar(None)
+        self.run_gtk_eventloop()
+        self.assertEqual(0, len(dialog._errored_entries))
+        self.assertNotEqual(
+            Gtk.STOCK_DIALOG_WARNING,
+            entry1.get_icon_stock(Gtk.EntryIconPosition.PRIMARY))
+
+    def test_validity_check_disabled(self):
+        """
+        Test that the L{validity_check} returns True when we disable landscape
+        client.
+        """
+        dialog = ClientSettingsDialog(self.controller)
+        self.run_gtk_eventloop()
         dialog.use_type_combobox.set_active(0)
         self.run_gtk_eventloop()
         self.assertTrue(dialog.validity_check())
 
-        # Check for hosted - currently returns True always
+    def test_validity_check_hosted(self):
+        """
+        Test that the L{validity_check} returns True when the hosted fields are
+        valid.
+        """
+        dialog = ClientSettingsDialog(self.controller)
+        self.run_gtk_eventloop()
         dialog.use_type_combobox.set_active(1)
+        dialog.hosted_account_name_entry.set_text("Bob")
+        dialog.hosted_password_entry.set_text("the builder")
         self.run_gtk_eventloop()
         self.assertTrue(dialog.validity_check())
 
-        # Checks for local
+    def test_validity_check_hosted_unicode(self):
+        """
+        Test that the L{validity_check} returns False when the hosted fields
+        contain Unicode.
+        """
+        dialog = ClientSettingsDialog(self.controller)
+        self.run_gtk_eventloop()
+        dialog.use_type_combobox.set_active(1)
+        dialog.hosted_account_name_entry.set_text(u"B\xc3b")
+        self.run_gtk_eventloop()
+        self.assertFalse(dialog.validity_check())
+
+    def test_validity_check_local_ok(self):
+        """
+        Test that the L{validity_check} returns True when the local fields
+        are valid.
+        """
+        dialog = ClientSettingsDialog(self.controller)
+        self.run_gtk_eventloop()
         dialog.use_type_combobox.set_active(2)
         self.run_gtk_eventloop()
         dialog.local_landscape_host_entry.set_text("foo.bar")
         self.run_gtk_eventloop()
         self.assertTrue(dialog.validity_check())
+
+    def test_validity_check_local_sanitisable(self):
+        """
+        Test that the L{validity_check} returns True when the local fields
+        are valid after sanitation.
+        """
+        dialog = ClientSettingsDialog(self.controller)
+        self.run_gtk_eventloop()
+        dialog.use_type_combobox.set_active(2)
         dialog.local_landscape_host_entry.set_text(" foo.bar")
         self.run_gtk_eventloop()
         self.assertTrue(dialog.validity_check())
         dialog.local_landscape_host_entry.set_text("foo.bar ")
         self.run_gtk_eventloop()
         self.assertTrue(dialog.validity_check())
+
+    def test_validity_check_local_invalid_host_name(self):
+        """
+        Test that the L{validity_check} returns False when the host name is
+        invalid.
+        """
+        dialog = ClientSettingsDialog(self.controller)
+        self.run_gtk_eventloop()
+        dialog.use_type_combobox.set_active(2)
         dialog.local_landscape_host_entry.set_text("foo bar")
         self.run_gtk_eventloop()
         self.assertFalse(dialog.validity_check())
+
+    def test_validity_check_local_unicode(self):
+        """
+        Test that the L{validity_check} returns False when the host name
+        contains Unicode.
+        """
+        dialog = ClientSettingsDialog(self.controller)
+        self.run_gtk_eventloop()
+        dialog.use_type_combobox.set_active(2)
         dialog.local_landscape_host_entry.set_text(u"f\xc3.bar")
         self.run_gtk_eventloop()
         self.assertFalse(dialog.validity_check())

=== modified file 'man/landscape-client.1'
--- man/landscape-client.1	2012-03-28 08:38:34 +0000
+++ man/landscape-client.1	2012-03-28 14:18:17 +0000
@@ -1,5 +1,5 @@
 .\"Text automatically generated by txt2man
-.TH landscape-client  "18 January 2010" "" ""
+.TH landscape-client 1 "27 March 2012" "" ""
 .SH NAME
 \fBlandscape-client \fP- Landscape system client
 \fB

=== modified file 'man/landscape-config.1'
--- man/landscape-config.1	2012-03-28 08:38:34 +0000
+++ man/landscape-config.1	2012-03-28 14:18:17 +0000
@@ -1,5 +1,5 @@
 .\"Text automatically generated by txt2man
-.TH landscape-config  "18 January 2010" "" ""
+.TH landscape-config 1 "27 March 2012" "" ""
 .SH NAME
 \fBlandscape-config \fP- configure the Landscape management client
 \fB

=== modified file 'man/landscape-message.1'
--- man/landscape-message.1	2012-03-28 08:38:34 +0000
+++ man/landscape-message.1	2012-03-28 14:18:17 +0000
@@ -1,5 +1,5 @@
 .\"Text automatically generated by txt2man
-.TH landscape-message  "18 January 2010" "" ""
+.TH landscape-message 1 "27 March 2012" "" ""
 .SH NAME
 \fBlandscape-message \fP- Send a message to the landscape web interface
 \fB

=== modified file 'po/fr.po'
--- po/fr.po	2012-03-28 08:38:34 +0000
+++ po/fr.po	2012-03-28 14:18:17 +0000
@@ -8,7 +8,7 @@
 msgstr ""
 "Project-Id-Version: PACKAGE VERSION\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2012-03-28 10:34+0200\n"
+"POT-Creation-Date: 2012-03-28 11:07+0200\n"
 "PO-Revision-Date: 2012-03-23 11:25+0100\n"
 "Last-Translator: Thomas Hervé <[e-mail]>\n"
 "Language-Team: français <>\n"
@@ -56,35 +56,39 @@
 msgid "Attempting to disable landscape client."
 msgstr "Tentative de désactivation du client Landscape."
 
-#: ../landscape/ui/view/configuration.py:27
+#: ../landscape/ui/view/configuration.py:55
 #: ../applications/landscape-client-settings.desktop.in.h:1
 msgid "Management Service"
 msgstr "Service de gestion"
 
-#: ../landscape/ui/view/configuration.py:66
+#: ../landscape/ui/view/configuration.py:124
 msgid "None"
 msgstr "Aucun"
 
-#: ../landscape/ui/view/configuration.py:70
+#: ../landscape/ui/view/configuration.py:128
 msgid "Landscape - hosted by Canonical"
 msgstr "Landscape - hébergé par Canonical"
 
-#: ../landscape/ui/view/configuration.py:74
+#: ../landscape/ui/view/configuration.py:132
 msgid "Landscape - dedicated server"
 msgstr "Landscape - serveur dédié"
 
-#: ../landscape/ui/view/configuration.py:78
+#: ../landscape/ui/view/configuration.py:136
 msgid "Register"
 msgstr "S'enregistrer"
 
-#: ../landscape/ui/view/configuration.py:82
+#: ../landscape/ui/view/configuration.py:140
 msgid "Disable"
 msgstr "Désactiver"
 
-#: ../landscape/ui/view/configuration.py:86
+#: ../landscape/ui/view/configuration.py:144
 msgid "Invalid host name."
 msgstr "Nom d'hôte invalide."
 
+#: ../landscape/ui/view/configuration.py:148
+msgid "Only ASCII characters are allowed."
+msgstr "Seuls les caractères ASCII sont autritorisés."
+
 #: ../landscape/ui/view/ui/landscape-client-settings.glade.h:1
 msgid ""
 "Landscape is a remote administration service from Canonical.  If you allow "

=== modified file 'po/landscape-client.pot'
--- po/landscape-client.pot	2012-03-28 08:38:34 +0000
+++ po/landscape-client.pot	2012-03-28 14:18:17 +0000
@@ -8,7 +8,7 @@
 msgstr ""
 "Project-Id-Version: PACKAGE VERSION\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2012-03-28 10:34+0200\n"
+"POT-Creation-Date: 2012-03-28 11:07+0200\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <[e-mail]>\n"
@@ -55,35 +55,39 @@
 msgid "Attempting to disable landscape client."
 msgstr ""
 
-#: ../landscape/ui/view/configuration.py:27
+#: ../landscape/ui/view/configuration.py:55
 #: ../applications/landscape-client-settings.desktop.in.h:1
 msgid "Management Service"
 msgstr ""
 
-#: ../landscape/ui/view/configuration.py:66
+#: ../landscape/ui/view/configuration.py:124
 msgid "None"
 msgstr ""
 
-#: ../landscape/ui/view/configuration.py:70
+#: ../landscape/ui/view/configuration.py:128
 msgid "Landscape - hosted by Canonical"
 msgstr ""
 
-#: ../landscape/ui/view/configuration.py:74
+#: ../landscape/ui/view/configuration.py:132
 msgid "Landscape - dedicated server"
 msgstr ""
 
-#: ../landscape/ui/view/configuration.py:78
+#: ../landscape/ui/view/configuration.py:136
 msgid "Register"
 msgstr ""
 
-#: ../landscape/ui/view/configuration.py:82
+#: ../landscape/ui/view/configuration.py:140
 msgid "Disable"
 msgstr ""
 
-#: ../landscape/ui/view/configuration.py:86
+#: ../landscape/ui/view/configuration.py:144
 msgid "Invalid host name."
 msgstr ""
 
+#: ../landscape/ui/view/configuration.py:148
+msgid "Only ASCII characters are allowed."
+msgstr ""
+
 #: ../landscape/ui/view/ui/landscape-client-settings.glade.h:1
 msgid ""
 "Landscape is a remote administration service from Canonical.  If you allow "