Merge lp:~tealeg/landscape-client/prevent-overwriting-config into lp:~landscape/landscape-client/trunk

[Geoff Teale]

This branch modifies the write method of
landscape.deployment.BaseConfiguration to ensure that valid
configuration values that existed in the file prior to modification
are retained even if they match defaults.

This branch modifies the write method of landscape.deployment.BaseConfiguration to ensure that valid configuration values that existed in the file prior to modification are retained even if they match defaults.

[Alberto Donato (ack)]

Great, +1!

review: Approve

[Chris Glass (tribaal)]

Nice! +1

review: Approve

=== modified file 'landscape/deployment.py'
--- landscape/deployment.py	2013-08-14 07:57:30 +0000
+++ landscape/deployment.py	2013-09-06 10:09:33 +0000
@@ -224,7 +224,8 @@
         all_options.update(self._set_options)
         for name, value in all_options.items():
             if name != "config" and name not in self.unsaved_options:
-                if value == self._command_line_defaults.get(name):
+                if (value == self._command_line_defaults.get(name) and
+                    name not in self._config_file_options):
                     config_parser.remove_option(self.config_section, name)
                 else:
                     config_parser.set(self.config_section, name, value)
 
=== modified file 'landscape/tests/test_configuration.py'
--- landscape/tests/test_configuration.py	2013-06-18 07:51:50 +0000
+++ landscape/tests/test_configuration.py	2013-09-06 10:09:33 +0000
@@ -749,6 +749,7 @@
 computer_title = rex
 data_path = %s
 account_name = account
+url = https://landscape.canonical.com/message-system
 """ % config.data_path)
 
     def test_silent_setup_no_register(self):
@@ -765,6 +766,7 @@
         self.assertConfigEqual(self.get_content(config), """\
 [client]
 data_path = %s
+url = https://landscape.canonical.com/message-system
 """ % config.data_path)
 
     def test_silent_setup_no_register_with_default_preseed_params(self):
@@ -801,6 +803,7 @@
             "account_name = \n"
             "computer_title = \n"
             "https_proxy = \n"
+            "url = https://landscape.canonical.com/message-system\n"
              % config.data_path)
 
     def test_silent_setup_without_computer_title(self):
 
=== modified file 'landscape/tests/test_deployment.py'
--- landscape/tests/test_deployment.py	2013-08-14 07:57:30 +0000
+++ landscape/tests/test_deployment.py	2013-09-06 10:09:33 +0000
@@ -175,12 +175,29 @@
             "[client]\nlog_level = warning\n")
 
     def test_dont_write_default_options(self):
+        """
+        Don't write options to the file if they exactly match the default and
+        didn't already exist in the file.
+        """
         self.write_config_file(log_level="debug")
         self.config.log_level = "info"
         self.config.write()
         data = open(self.config_filename).read()
         self.assertConfigEqual(data, "[client]")
 
+    def test_do_write_preexisting_default_options(self):
+        """
+        If the value of an option matches the default, but the option was
+        already written in the file, then write it back to the file.
+        """
+        config = "[client]\nlog_level = info\n"
+        config_filename = self.makeFile(config)
+        self.config.load_configuration_file(config_filename)
+        self.config.log_level = "info"
+        self.config.write()
+        data = open(config_filename).read()
+        self.assertConfigEqual(data, "[client]\nlog_level = info\n")
+
     def test_dont_delete_explicitly_set_default_options(self):
         """
         If the user explicitly sets a configuration option to its default