some tests

test/test_config.py

@@ -169,7 +169,7 @@ def test_config_impl_get_path_by_opt():
     unknown
     assert config.cfgimpl_get_description().impl_get_path_by_opt(boo) == 'bool'
     assert config.cfgimpl_get_description().impl_get_path_by_opt(dummy) == 'gc.dummy'
-    raises(AttributeError, "config.cfgimpl_get_description().impl_get_path_by_opt(unknown)")
+    #raises(AttributeError, "config.cfgimpl_get_description().impl_get_path_by_opt(unknown)")
 
 
 #def test_config_impl_get_path_by_opt_od():

test/test_config_api.py

@@ -15,7 +15,7 @@ def make_description():
     gcoption = ChoiceOption('name', 'GC name', ('ref', 'framework'), 'ref')
     gcdummy = BoolOption('dummy', 'dummy', default=False)
     prop = BoolOption('prop', 'prop 1', properties=('disabled',))
-    prop2 = BoolOption('prop', 'prop 2', properties=('hidden',))
+    prop2 = StrOption('prop', 'prop 2', properties=('hidden',))
     objspaceoption = ChoiceOption('objspace', 'Object space',
                                   ('std', 'thunk'), 'std')
     booloption = BoolOption('bool', 'Test boolean option', default=True)
@@ -74,6 +74,11 @@ def _is_same_opt(opt1, opt2):
 #        assert value == gvalue
 
 
+def test_od_not_list():
+    b = BoolOption('bool', '', multi=True)
+    raises(ValueError, "OptionDescription('od', '', b)")
+
+
 def test_str():
     descr = make_description()
     c = Config(descr)
@@ -230,12 +235,11 @@ def test_find_in_config():
     _is_same_opt(ret[0].option.get(), api.forcepermissive.option('gc.prop').option.get())
     #
     _is_same_opt(api.forcepermissive.option.find('prop', first=True).option.get(), api.forcepermissive.option('gc.prop').option.get())
-    #FIXME cannot find an option without name
     # combinaison of filters
-    #ret = conf.find(bytype=BoolOption, byname='dummy')
+    ret = api.unrestraint.option.find('prop', type=BoolOption)
-    #assert len(ret) == 1
+    assert len(ret) == 1
-    #_is_same_opt(ret[0], conf.unwrap_from_path('gc.dummy'))
+    _is_same_opt(ret[0].option.get(), api.unrestraint.option('gc.gc2.prop').option.get())
-    #_is_same_opt(conf.find_first(bytype=BoolOption, byname='dummy'), conf.unwrap_from_path('gc.dummy'))
+    _is_same_opt(api.unrestraint.option.find('prop', type=BoolOption, first=True).option.get(), api.unrestraint.option('gc.gc2.prop').option.get())
     #
     ret = api.option.find('dummy', value=False)
     assert len(ret) == 1

test/test_dyn_optiondescription.py

@@ -493,6 +493,14 @@ def test_decrease_dyndescription_context():
     raises(AttributeError, "api.option('od.dodval2.stval2').value.get()")
 
 
+def test_dyndescription_root():
+    boolean = BoolOption('boolean', '', True)
+    st1 = StrOption('st', '', requires=[{'option': boolean, 'expected': False,
+                                        'action': 'disabled'}])
+    dod = DynOptionDescription('dod', '', [boolean, st1], callback=return_list)
+    raises(ConfigError, "Config(dod)")
+
+
 def test_requires_dyndescription():
     boolean = BoolOption('boolean', '', True)
     st1 = StrOption('st', '', requires=[{'option': boolean, 'expected': False,

test/test_masterslaves.py

@@ -3,7 +3,7 @@ from .autopath import do_autopath
 do_autopath()
 
 from tiramisu.setting import groups, owners
-from tiramisu import ChoiceOption, BoolOption, IntOption, \
+from tiramisu import ChoiceOption, BoolOption, IntOption, IPOption, NetmaskOption, \
     StrOption, OptionDescription, MasterSlaves, Config, getapi
 from tiramisu.error import SlaveError, PropertiesOptionError, APIError, ConfigError
 from tiramisu.api import TIRAMISU_VERSION
@@ -644,3 +644,39 @@ def test_slave_not_multi():
     ip_admin_eth0 = StrOption('ip_admin_eth0', "ip réseau autorisé", multi=True)
     netmask_admin_eth0 = StrOption('netmask_admin_eth0', "masque du sous-réseau")
     raises(ValueError, "MasterSlaves('ip_admin_eth0', '', [ip_admin_eth0, netmask_admin_eth0])")
+
+
+def test_slave_not_same():
+    ip_admin_eth0 = IPOption('ip_admin_eth0', "ip réseau autorisé", multi=True, default=['1.1.1.1'])
+    netmask_admin_eth0 = NetmaskOption('netmask_admin_eth0', "masque du sous-réseau", multi=True)
+    interface0 = MasterSlaves('interface0', '', [ip_admin_eth0, netmask_admin_eth0])
+    ip_admin_eth1 = IPOption('ip_admin_eth1', "ip réseau autorisé", multi=True, default=['1.1.1.1'])
+    netmask_admin_eth1 = NetmaskOption('netmask_admin_eth1', "masque du sous-réseau", multi=True)
+    netmask_admin_eth1.impl_add_consistency('ip_netmask', ip_admin_eth0)
+    interface1 = MasterSlaves('interface1', '', [ip_admin_eth1, netmask_admin_eth1])
+    od1 = OptionDescription('od', '', [interface0, interface1])
+    maconfig = OptionDescription('toto', '', [od1])
+    raises(ConfigError, "getapi(Config(maconfig))")
+
+
+def test_slave_not_same_not_equal():
+    ip_admin_eth0 = IPOption('ip_admin_eth0', "ip réseau autorisé", multi=True, default=['1.1.1.1'])
+    netmask_admin_eth0 = NetmaskOption('netmask_admin_eth0', "masque du sous-réseau", multi=True)
+    interface0 = MasterSlaves('interface0', '', [ip_admin_eth0, netmask_admin_eth0])
+    ip_admin_eth1 = IPOption('ip_admin_eth1', "ip réseau autorisé", multi=True, default=['1.1.1.1'])
+    netmask_admin_eth1 = NetmaskOption('netmask_admin_eth1', "masque du sous-réseau", multi=True)
+    netmask_admin_eth1.impl_add_consistency('not_equal', netmask_admin_eth0)
+    interface1 = MasterSlaves('interface1', '', [ip_admin_eth1, netmask_admin_eth1])
+    od1 = OptionDescription('od', '', [interface0, interface1])
+    maconfig = OptionDescription('toto', '', [od1])
+    api = getapi(Config(maconfig))
+    api.property.read_write()
+
+
+def test_slave_force_store_value():
+    ip_admin_eth0 = IPOption('ip_admin_eth0', "ip réseau autorisé", multi=True, default=['1.1.1.1'])
+    netmask_admin_eth0 = NetmaskOption('netmask_admin_eth0', "masque du sous-réseau", multi=True, properties=('force_store_value',))
+    interface0 = MasterSlaves('interface0', '', [ip_admin_eth0, netmask_admin_eth0])
+    od1 = OptionDescription('od', '', [interface0])
+    maconfig = OptionDescription('toto', '', [od1])
+    raises(ConfigError, "getapi(Config(maconfig))")

test/test_submulti.py

@@ -27,6 +27,10 @@ def return_list2(value=None):
     return [['val', 'val']]
 
 
+def test_unknown_multi():
+    raises(ValueError, "StrOption('multi', '', multi='unknown')")
+
+
 def test_submulti():
     multi = StrOption('multi', '', multi=submulti)
     if TIRAMISU_VERSION == 2:
@@ -45,6 +49,10 @@ def test_submulti():
     assert api.option('multi').owner.get() == owners.default
 
 
+def test_submulti_default_multi_not_list():
+    raises(ValueError, "StrOption('multi2', '', default_multi='yes', multi=submulti)")
+
+
 def test_append_submulti():
     multi = StrOption('multi', '', multi=submulti)
     if TIRAMISU_VERSION == 2:
@@ -390,44 +398,53 @@ def test_submulti_unique():
     api.option('int').value.set([[0], [0]])
     raises(ValueError, "api.option('int').value.set([[1, 0, 2, 3, 4, 5, 6, 0, 7], [0]])")
     api.option('int').value.set([[0, 4, 5, 6], [0]])
-#
+
-#
+
-#def test_multi_submulti_meta():
+def test_submulti_unknown_unique():
-#    multi = StrOption('multi', '', multi=submulti)
+    raises(ValueError, "IntOption('int', '', multi=submulti, unique='str')")
-#    od = OptionDescription('od', '', [multi])
+
-#    conf1 = Config(od, session_id='conf1')
+
-#    api1 = getapi(conf1)
+def test_unique_not_multi():
-#    api1.property.read_write()
+    raises(ValueError, "IntOption('int', '', unique=True)")
-#    conf2 = Config(od, session_id='conf2')
+
-#    api2 = getapi(conf2)
+
-#    api2.property.read_write()
+def test_multi_submulti_meta():
-#    meta = MetaConfig([conf1, conf2])
+    multi = StrOption('multi', '', multi=submulti)
-#    api3 = getapi(meta)
+    od = OptionDescription('od', '', [multi])
-#    api3.property.read_write()
+    conf1 = Config(od, session_id='conf1')
-#    api3.option('multi').value.set([['val']])
+    api1 = getapi(conf1)
-#    assert api3.option('multi').value.get() == [['val']]
+    api1.property.read_write()
-#    api3.config('conf1').option('multi').value.set([['val', None]])
+    conf2 = Config(od, session_id='conf2')
-#    assert api1.option('multi').value.get() == [['val', None]]
+    api2 = getapi(conf2)
-#    assert api3.config('conf1').option('multi').value.get() == [['val', None]]
+    api2.property.read_write()
-#    assert api3.option('multi').value.get() == [['val']]
+    meta = MetaConfig([conf1, conf2])
-#
+    api3 = getapi(meta)
-#
+    api3.property.read_write()
-#def test_multi_submulti_meta_no_cache():
+    api3.option('multi').value.set([['val']])
-#    multi = StrOption('multi', '', multi=submulti)
+    assert api3.option('multi').value.get() == [['val']]
-#    od = OptionDescription('od', '', [multi])
+    api3.config('conf1').option('multi').value.set([['val', None]])
-#    conf1 = Config(od, session_id='conf1_1')
+    assert api1.option('multi').value.get() == [['val', None]]
-#    api1 = getapi(conf1)
+    assert api3.config('conf1').option('multi').value.get() == [['val', None]]
-#    api1.property.read_write()
+    assert api3.option('multi').value.get() == [['val']]
-#    conf2 = Config(od, session_id='conf2_1')
+
-#    api2 = getapi(conf2)
+
-#    api2.property.read_write()
+def test_multi_submulti_meta_no_cache():
-#    meta = MetaConfig([conf1, conf2])
+    multi = StrOption('multi', '', multi=submulti)
-#    api3 = getapi(api2)
+    multi = StrOption('multi', '', multi=submulti)
-#    api3.property.read_write()
+    od = OptionDescription('od', '', [multi])
-#    api3.property.pop('cache')
+    conf1 = Config(od, session_id='conf1')
-#    api3.option('multi').value.set([['val']])
+    api1 = getapi(conf1)
-#    assert api3.option('multi').value.get() == [['val']]
+    api1.property.read_write()
-#    api3.config('conf1').option('multi').value.set([['val', None]])
+    conf2 = Config(od, session_id='conf2')
-#    assert api1.option('multi').value.get() == [['val', None]]
+    api2 = getapi(conf2)
-#    assert api3.config('conf1').option('multi').value.get() == [['val', None]]
+    api2.property.read_write()
-#    assert api3.option('multi').value.get() == [['val']]
+    meta = MetaConfig([conf1, conf2])
+    api3 = getapi(meta)
+    api3.property.read_write()
+    api3.property.pop('cache')
+    api3.option('multi').value.set([['val']])
+    assert api3.option('multi').value.get() == [['val']]
+    api3.config('conf1').option('multi').value.set([['val', None]])
+    assert api1.option('multi').value.get() == [['val', None]]
+    assert api3.config('conf1').option('multi').value.get() == [['val', None]]
+    assert api3.option('multi').value.get() == [['val']]

tiramisu/api.py

@@ -660,13 +660,14 @@ class TiramisuOption(CommonTiramisu):
     def _find(self,
               name,
               value=undefined,
+              type=None,
               first=False):
         """find an option by name (only for optiondescription)"""
         if not first:
             ret = []
         for path in self.config_bag.config.find(byname=name,
                                                 byvalue=value,
-                                                bytype=None,
+                                                bytype=type,
                                                 _subpath=self._path,
                                                 config_bag=self.config_bag):
             config_bag = self.config_bag.copy('nooption')
@@ -897,13 +898,14 @@ class TiramisuContextOption(TiramisuContext):
     def find(self,
              name,
              value=undefined,
+              type=None,
              first=False):
         """find an option by name"""
         if not first:
             ret = []
         for path in self.config_bag.config.find(byname=name,
                                                 byvalue=value,
-                                                bytype=None,
+                                                bytype=type,
                                                 #_subpath=self._path,
                                                 config_bag=self.config_bag):
             config_bag = self.config_bag.copy('nooption')

tiramisu/config.py

@@ -26,6 +26,7 @@ from copy import copy
 
 from .error import PropertiesOptionError, ConfigError, ConflictError, SlaveError
 from .option.syndynoptiondescription import SynDynOptionDescription
+from .option.dynoptiondescription import DynOptionDescription
 from .option.masterslave import MasterSlaves
 from .option.baseoption import BaseOption, valid_name
 from .setting import ConfigBag, groups, Settings, undefined
@@ -716,6 +717,8 @@ class Config(_CommonConfig):
         self._impl_meta = None
         if isinstance(descr, MasterSlaves):
             raise ConfigError(_('cannot set masterslaves object has root optiondescription'))
+        if isinstance(descr, DynOptionDescription):
+            raise ConfigError(_('cannot set dynoptiondescription object has root optiondescription'))
         if force_settings is not None and force_values is not None:
             if isinstance(force_settings, tuple):
                 self._impl_settings = Settings(self,

tiramisu/option/option.py

@@ -92,7 +92,7 @@ class Option(OnlyOption):
             is_multi = True
             _multi = submulti
         else:
-            raise ValueError(_('invalid multi value'))
+            raise ValueError(_('invalid multi type "{}"').format(multi))
         if _multi != 1:
             _setattr(self, '_multi', _multi)
         if multi is not False and default is None:
@@ -118,7 +118,7 @@ class Option(OnlyOption):
         if extra is not None:
             _setattr(self, '_extra', extra)
         if unique != undefined and not isinstance(unique, bool):
-            raise ValueError(_('unique must be a boolean'))
+            raise ValueError(_('unique must be a boolean, not "{}"').format(unique))
         if not is_multi and unique is True:
             raise ValueError(_('unique must be set only with multi value'))
         if warnings_only is True:
@@ -137,10 +137,10 @@ class Option(OnlyOption):
                                                                      str(err)))
             if _multi is submulti:
                 if not isinstance(default_multi, list):
-                    raise ValueError(_("invalid default_multi value {0} "
+                    raise ValueError(_('invalid default_multi value "{0}" '
-                                       "for option {1}: must be a list for a submulti"
+                                       'for option "{1}", must be a list for a submulti'
-                                       "").format(str(default_multi),
+                                       '').format(str(default_multi),
-                                                  self.impl_getname()))
+                                                 self.impl_get_display_name()))
                 for value in default_multi:
                     test_multi_value(value)
             else:

tiramisu/option/optiondescription.py

@@ -19,7 +19,6 @@
 # the whole pypy projet is under MIT licence
 # ____________________________________________________________
 from copy import copy
-from itertools import chain
 
 
 from ..i18n import _
@@ -39,8 +38,7 @@ class CacheOptionDescription(BaseOption):
                      _consistencies=None,
                      _consistencies_id=0,
                      cache_option=None,
-                     force_store_values=None,
+                     force_store_values=None):
-                     _dependencies=None):
         """validate duplicate option and set option has readonly option
         """
         # cache_option is None only when we start to build cache
@@ -52,7 +50,6 @@ class CacheOptionDescription(BaseOption):
             _consistencies = {}
             cache_option = []
             force_store_values = []
-            _dependencies = []
         else:
             init = False
 
@@ -70,8 +67,7 @@ class CacheOptionDescription(BaseOption):
                                     _consistencies,
                                     _consistencies_id,
                                     cache_option,
-                                    force_store_values,
+                                    force_store_values)
-                                    _dependencies)
             else:
                 option._set_readonly()
                 is_multi = option.impl_is_multi()
@@ -91,7 +87,7 @@ class CacheOptionDescription(BaseOption):
                         is_masterslaves = option.impl_is_master_slaves()
                         if not is_masterslaves:
                             raise ConfigError(_('malformed consistency option "{0}" '
-                                               'must be a master/slaves').format(
+                                               'must be a masterslaves').format(
                                                    option.impl_getname()))
                         masterslaves = option.impl_get_master_slaves()
                     for weak_opt in all_cons_opts:
@@ -103,7 +99,7 @@ class CacheOptionDescription(BaseOption):
                                                        option.impl_getname(), opt.impl_getname()))
                             elif masterslaves != opt.impl_get_master_slaves():
                                 raise ConfigError(_('malformed consistency option "{0}" '
-                                                   'must be in same master/slaves for "{1}"').format(
+                                                   'must be in same masterslaves as "{1}"').format(
                                                        option.impl_getname(), opt.impl_getname()))
                         _consistencies.setdefault(weak_opt,
                                                   []).append((_consistencies_id,
@@ -123,7 +119,7 @@ class CacheOptionDescription(BaseOption):
                                 #if option in require is a multi:
                                 # * option in require must be a master or a slave
                                 # * current option must be a slave (and only a slave)
-                                # * option in require and current option must be in same master/slaves
+                                # * option in require and current option must be in same masterslaves
                                 for require_opt, values in require[0]:
                                     if require_opt.impl_is_multi():
                                         if is_slave is None:
@@ -132,8 +128,8 @@ class CacheOptionDescription(BaseOption):
                                                 masterslaves = option.impl_get_master_slaves()
                                         if is_slave and require_opt.impl_is_master_slaves():
                                             if masterslaves != require_opt.impl_get_master_slaves():
-                                                raise ValueError(_('malformed requirements option {0} '
+                                                raise ValueError(_('malformed requirements option "{0}" '
-                                                                   'must be in same master/slaves for {1}').format(
+                                                                   'must be in same masterslaves for "{1}"').format(
                                                                        require_opt.impl_getname(), option.impl_getname()))
                                         else:
                                             raise ValueError(_('malformed requirements option "{0}" '
@@ -149,14 +145,12 @@ class CacheOptionDescription(BaseOption):
                 self._cache_consistencies = {}
                 for weak_opt, cons in _consistencies.items():
                     opt = weak_opt()
-                    if opt not in cache_option:  # pragma: optional cover
+                    if opt not in cache_option:
                         raise ConfigError(_('consistency with option {0} '
                                             'which is not in Config').format(
                                                 opt.impl_getname()))
                     self._cache_consistencies[opt] = tuple(cons)
             self._cache_force_store_values = force_store_values
-            if _dependencies:
-                self._dependencies = tuple(_dependencies)
             self._set_readonly()
 
     def impl_already_build_caches(self):
@@ -171,11 +165,11 @@ class CacheOptionDescription(BaseOption):
         for subpath, option in self._cache_force_store_values:
             if option.impl_is_master_slaves('slave'):
                 # problem with index
-                raise ConfigError(_('a slave ({0}) cannot have '
+                raise ConfigError(_('the slave "{0}" cannot have '
-                                    'force_store_value property').format(subpath))
+                                    '"force_store_value" property').format(option.impl_get_display_name()))
             if option.issubdyn():
-                raise ConfigError(_('a dynoption ({0}) cannot have '
+                raise ConfigError(_('the dynoption "{0}" cannot have '
-                                    'force_store_value property').format(subpath))
+                                    '"force_store_value" property').format(option.impl_get_display_name()))
             if not values._p_.hasvalue(subpath):
                 config_bag = ConfigBag(config=context, option=option)
                 value = values.getvalue(subpath,
@@ -231,15 +225,8 @@ class OptionDescriptionWalk(CacheOptionDescription):
                          config_bag):
         dynopt = option.getsubdyn()
         rootpath = self.impl_get_path_by_opt(dynopt)
-        if rootpath == '':
+        ori_index = len(rootpath) + 1
-            ori_index = 0
+        subpaths = [rootpath] + option.impl_getpath(config_bag.config)[ori_index:].split('.')[:-1]
-        else:
-            ori_index = len(rootpath) + 1
-        subpaths = option.impl_getpath(config_bag.config)[ori_index:].split('.')[:-1]
-        if subpaths != ['']:
-            subpaths = [rootpath] + subpaths
-        else:
-            subpaths = [rootpath]
         for suffix in dynopt._impl_get_suffixes(config_bag):
             subpath = '.'.join([subp + suffix for subp in subpaths])
             if isinstance(option, OnlyOption):
@@ -260,13 +247,7 @@ class OptionDescriptionWalk(CacheOptionDescription):
                             option):
             if isinstance(option,
                           bytype):
-                if byname is None:
+                return _filter_by_name(path, option)
-                    if option.issubdyn():
-                        for doption in self.build_dynoptions(option, config_bag):
-                            dpath = doption.impl_getname(config_bag.config)
-                            yield (dpath, doption)
-                    else:
-                        yield (path, option)
 
         def _filter_by_name(path,
                             option):
@@ -276,22 +257,14 @@ class OptionDescriptionWalk(CacheOptionDescription):
                     for doption in self.build_dynoptions(option, config_bag):
                         if byname == doption.impl_getname():
                             dpath = doption.impl_getpath(config_bag.config)
-                            yield (dpath, doption)
+                            return (dpath, doption)
-                            break
+            elif byname == name:
-            else:
+                return (path, option)
-                if byname == name:
-                    yield (path, option)
 
         def _filter(path, option):
-            generators = []
             if bytype is not None:
-                generators.append(_filter_by_type(path, option))
+                return _filter_by_type(path, option)
-            if byname is not None:
+            return _filter_by_name(path, option)
-                generators.append(_filter_by_name(path, option))
-            if len(generators) == 1:
-                return generators[0]
-            else:
-                return chain(*generators)
 
         opts, paths = self._cache_paths
         for index, option in enumerate(opts):
@@ -300,16 +273,9 @@ class OptionDescriptionWalk(CacheOptionDescription):
             path = paths[index]
             if _subpath is not None and not path.startswith(_subpath + '.'):
                 continue
-            if bytype == byname is None:
+            ret = _filter(path, option)
-                if option.issubdyn():
+            if ret:
-                    for doption in self.build_dynoptions(option, config_bag):
+                yield ret
-                        dpath = doption.impl_getpath(config_bag.config)
-                        yield (dpath, doption)
-                else:
-                    yield (dpath, option)
-            else:
-                for ret in _filter(path, option):
-                    yield ret
 
     def impl_getchild(self,
                       name,
@@ -331,18 +297,10 @@ class OptionDescriptionWalk(CacheOptionDescription):
 
     def impl_get_opt_by_path(self,
                              path):
-        if getattr(self, '_cache_paths', None) is None:
-            raise ConfigError(_('use impl_get_opt_by_path only with root optiondescription'))
-        if path not in self._cache_paths[1]:
-            raise AttributeError(_('no option for path "{}"').format(path))
         return self._cache_paths[0][self._cache_paths[1].index(path)]
 
     def impl_get_path_by_opt(self,
                              opt):
-        if getattr(self, '_cache_paths', None) is None:
-            raise ConfigError(_('use impl_get_path_by_opt only with root OptionDescription'))
-        if opt not in self._cache_paths[0]:
-            raise AttributeError(_('no option "{}" found').format(opt))
         return self._cache_paths[1][self._cache_paths[0].index(opt)]
 
     def impl_getchildren(self,
@@ -351,7 +309,7 @@ class OptionDescriptionWalk(CacheOptionDescription):
         subpath = None
         for child in self._impl_st_getchildren():
             if dyn and child.impl_is_dynoptiondescription():
-                if config_bag.config is None:
+                if config_bag.config is None:  # pragma: no cover
                     raise ConfigError(_('need context'))
                 if subpath is None:
                     if config_bag.config.cfgimpl_get_description() == self:
@@ -437,7 +395,7 @@ class OptionDescription(OptionDescriptionWalk):
         valid_child.sort()
         old = None
         for child in valid_child:
-            if child == old:  # pragma: optional cover
+            if child == old:
                 raise ConflictError(_('duplicate option name: '
                                       '"{0}"').format(child))
             if dynopt_names:
@@ -471,7 +429,7 @@ class OptionDescription(OptionDescriptionWalk):
         :param group_type: an instance of `GroupType` or `MasterGroupType`
                               that lives in `setting.groups`
         """
-        if self._group_type != groups.default:  # pragma: optional cover
+        if self._group_type != groups.default:
             raise TypeError(_('cannot change group_type if already set '
                               '(old {0}, new {1})').format(self._group_type,
                                                            group_type))