New config: More powerful :config- commands: add #4283

Made minor changes to the second commit which broke tests out into
success and failure tests taking advantage of pytests.raises.

Additionally updated several grammar issues.

Continues #2794

qutebrowser/config/configcommands.py

@@ -245,7 +245,7 @@ class ConfigCommands:
 
         Args:
             option: The name of the option.
-            temp: Don't touch autoconfig.yml.
+            temp: Set value temporarily until qutebrowser is closed.
         """
         with self._handle_config_error():
             self._config.unset(option, save_yaml=not temp)
@@ -255,12 +255,10 @@ class ConfigCommands:
     def config_add_list(self, option, value, temp=False):
         """Append a value to a config option that is a list.
 
-        This appends an option to a config setting that is a list.
-
         Args:
             option: The name of the option.
-            value: The value to append to the end of the dictionary.
-            temp: Don't touch autoconfig.yml.
+            value: The value to append to the end of the list.
+            temp: Set value temporarily until qutebrowser is closed.
         """
         opt = self._config.get_opt(option)
         valid_list_types = (configtypes.List, configtypes.ListOrValue)
@@ -290,15 +288,15 @@ class ConfigCommands:
         """
         opt = self._config.get_opt(option)
         if not isinstance(opt.typ, configtypes.Dict):
-            raise cmdexc.CommandError(":config-add-list can only be used for "
+            raise cmdexc.CommandError(":config-add-dict can only be used for "
                                       "dicts")
 
         with self._handle_config_error():
             option_value = self._config.get_mutable_obj(option)
 
             if key in option_value and not replace:
-                raise cmdexc.CommandError(("{} already existed in {} - use "
-                                           "--replace to overwrite!")
+                raise cmdexc.CommandError("{} already exists in {} - use "
+                                          "--replace to overwrite!"
                                           .format(key, option))
 
             option_value[key] = value

tests/unit/config/test_configcommands.py

@@ -25,7 +25,7 @@ import unittest.mock
 import pytest
 from PyQt5.QtCore import QUrl
 
-from qutebrowser.config import configcommands, configtypes, configutils
+from qutebrowser.config import configcommands, configutils
 from qutebrowser.commands import cmdexc
 from qutebrowser.utils import usertypes, urlmatch
 from qutebrowser.keyinput import keyutils
@@ -286,20 +286,12 @@ class TestAdd:
 
     """Test :config-add-list and :config-add-dict."""
 
-    @pytest.mark.parametrize('name', ['content.host_blocking.whitelist',
-                                      'history_gap_interval'])
     @pytest.mark.parametrize('temp', [True, False])
-    @pytest.mark.parametrize('value', ['test1', 'test2', '', None])
-    def test_add_list(self, commands, config_stub, yaml_value, name, temp, value):
-        opt_type = config_stub.get_opt(name).typ
+    @pytest.mark.parametrize('value', ['test1', 'test2'])
+    def test_add_list(self, commands, config_stub, yaml_value, temp, value):
+        name = 'content.host_blocking.whitelist'
 
-        try:
-            commands.config_add_list(name, value, temp=temp)
-        except cmdexc.CommandError:
-            # We attempted to add to the dictionary with replace as false.
-            valid_list_types = (configtypes.List, configtypes.ListOrValue)
-            assert not isinstance(opt_type, valid_list_types) or not value
-            return
+        commands.config_add_list(name, value, temp=temp)
 
         assert str(config_stub.get(name)[-1]) == value
         if temp:
@@ -307,21 +299,37 @@ class TestAdd:
         else:
             assert yaml_value(name)[-1] == value
 
-    @pytest.mark.parametrize('name', ['aliases', 'history_gap_interval'])
-    @pytest.mark.parametrize('key', ['w', 'missingkey'])
+    def test_add_list_non_list(self, commands):
+        name = 'history_gap_interval'
+        value = 'value'
+        with pytest.raises(
+                cmdexc.CommandError,
+                match=":config-add-list can only be used for lists"):
+            commands.config_add_list(name, value)
+
+    def test_add_list_empty_value(self, commands):
+        name = 'content.host_blocking.whitelist'
+        value = ''
+        with pytest.raises(
+                cmdexc.CommandError,
+                match="Invalid value '{}' - may not be empty!".format(value)):
+            commands.config_add_list(name, value)
+
+    def test_add_list_none_value(self, commands):
+        name = 'content.host_blocking.whitelist'
+        value = None
+        with pytest.raises(
+                cmdexc.CommandError,
+                match="Invalid value 'None' - may not be null!"):
+            commands.config_add_list(name, value)
+
     @pytest.mark.parametrize('value', ['test1', 'test2'])
     @pytest.mark.parametrize('temp', [True, False])
-    @pytest.mark.parametrize('replace', [True, False])
-    def test_add_dict(self, commands, config_stub, yaml_value, name, key,
-                      value, temp, replace):
-        opt_type = config_stub.get_opt(name).typ
+    def test_add_dict(self, commands, config_stub, yaml_value, value, temp):
+        name = 'aliases'
+        key = 'missingkey'
 
-        try:
-            commands.config_add_dict(name, key, value, temp=temp, replace=replace)
-        except cmdexc.CommandError:
-            # We attempted to add to the dictionary with replace as false.
-            assert not isinstance(opt_type, configtypes.Dict) or not replace
-            return
+        commands.config_add_dict(name, key, value, temp=temp)
 
         assert str(config_stub.get(name)[key]) == value
         if temp:
@@ -329,6 +337,47 @@ class TestAdd:
         else:
             assert yaml_value(name)[key] == value
 
+    @pytest.mark.parametrize('replace', [True, False])
+    def test_add_dict_replace(self, commands, config_stub, replace):
+        name = 'aliases'
+        key = 'w'
+        value = 'anything'
+
+        if replace:
+            commands.config_add_dict(name, key, value, replace=True)
+            assert str(config_stub.get(name)[key]) == value
+        else:
+            with pytest.raises(
+                    cmdexc.CommandError,
+                    match="w already exists in aliases - use --replace to "
+                          "overwrite!"):
+                commands.config_add_dict(name, key, value, replace=False)
+
+    def test_add_dict_non_dict(self, commands):
+        name = 'history_gap_interval'
+        key = 'value'
+        value = 'value'
+        with pytest.raises(
+                cmdexc.CommandError,
+                match=":config-add-dict can only be used for dicts"):
+            commands.config_add_dict(name, key, value)
+
+    def test_add_dict_empty_value(self, commands):
+        name = 'aliases'
+        key = 'missingkey'
+        value = ''
+        with pytest.raises(cmdexc.CommandError,
+                           match="Invalid value '' - may not be empty!"):
+            commands.config_add_dict(name, key, value)
+
+    def test_add_dict_none_value(self, commands):
+        name = 'aliases'
+        key = 'missingkey'
+        value = None
+        with pytest.raises(cmdexc.CommandError,
+                           match="Invalid value 'None' - may not be null!"):
+            commands.config_add_dict(name, key, value)
+
 
 class TestUnsetAndClear: