From cd7977eae6c11e02e6456f13438dc83dc334184c Mon Sep 17 00:00:00 2001 From: Emmanuel Garette Date: Wed, 12 Sep 2018 21:05:14 +0200 Subject: [PATCH] coverage --- test/test_option.py | 36 +++++++++++++++++++++- test/test_option_callback.py | 14 +++++++++ test/test_option_consistency.py | 45 +++++++++++++++------------- test/test_option_owner.py | 11 +++++++ tiramisu/option/broadcastoption.py | 3 +- tiramisu/option/choiceoption.py | 1 - tiramisu/option/ipoption.py | 8 +++-- tiramisu/option/netmaskoption.py | 18 ++++++----- tiramisu/option/option.py | 16 +++++----- tiramisu/option/optiondescription.py | 28 +++++++---------- tiramisu/setting.py | 3 ++ tiramisu/storage/util.py | 22 +++++++------- tiramisu/value.py | 16 +++------- 13 files changed, 140 insertions(+), 81 deletions(-) diff --git a/test/test_option.py b/test/test_option.py index 4b77356..1f4a340 100644 --- a/test/test_option.py +++ b/test/test_option.py @@ -6,8 +6,9 @@ do_autopath() from py.test import raises -from tiramisu.error import APIError +from tiramisu.error import APIError, ConfigError from tiramisu import IntOption, OptionDescription, Config +from tiramisu.setting import groups def a_func(): @@ -113,6 +114,39 @@ def test_unknown_option(): raises(AttributeError, "api.option('od.unknown.suboption').value.get()") +def test_optiondescription_group(): + groups.notfamily = groups.GroupType('notfamily') + i = IntOption('test', '') + i2 = IntOption('test', '') + od1 = OptionDescription('od', '', [i]) + od1.impl_set_group_type(groups.family) + od3 = OptionDescription('od2', '', [i2]) + od3.impl_set_group_type(groups.notfamily) + od2 = OptionDescription('od', '', [od1, od3]) + api = Config(od2) + assert len(list(api.option.list('optiondescription'))) == 2 + assert len(list(api.option.list('optiondescription', group_type=groups.family))) == 1 + assert len(list(api.option.list('optiondescription', group_type=groups.notfamily))) == 1 + + +def test_optiondescription_group_redefined(): + try: + groups.notfamily = groups.GroupType('notfamily') + except: + pass + i = IntOption('test', '') + od1 = OptionDescription('od', '', [i]) + od1.impl_set_group_type(groups.family) + raises(ValueError, "od1.impl_set_group_type(groups.notfamily)") + + +def test_optiondescription_group_masterslave(): + i = IntOption('test', '') + od1 = OptionDescription('od', '', [i]) + raises(ConfigError, "od1.impl_set_group_type(groups.master)") + + + def test_asign_optiondescription(): i = IntOption('test', '') od1 = OptionDescription('od', '', [i]) diff --git a/test/test_option_callback.py b/test/test_option_callback.py index e520e3a..d1883f6 100644 --- a/test/test_option_callback.py +++ b/test/test_option_callback.py @@ -241,6 +241,20 @@ def test_callback_params_without_callback(): raises(ValueError, "StrOption('val2', '', callback_params=Params(ParamValue('yes')))") +def test_params(): + raises(ValueError, "Params([ParamContext()])") + raises(ValueError, "Params('str')") + raises(ValueError, "Params(('str',))") + raises(ValueError, "Params(kwargs=[ParamContext()])") + raises(ValueError, "Params(kwargs={'a': 'str'})") + + +def test_param_option(): + val1 = StrOption('val1', "") + raises(ValueError, "ParamOption('str')") + raises(ValueError, "ParamOption(val1, 'str')") + + def test_callback_invalid(): raises(ValueError, 'val1 = StrOption("val1", "", callback="string")') raises(ValueError, 'val1 = StrOption("val1", "", callback=return_val, callback_params="string")') diff --git a/test/test_option_consistency.py b/test/test_option_consistency.py index a37977b..d1054fb 100644 --- a/test/test_option_consistency.py +++ b/test/test_option_consistency.py @@ -415,6 +415,17 @@ def test_consistency_ip_netmask(): api.option('b').value.set('255.255.255.0') raises(ValueError, "api.option('a').value.set('192.168.1.0')") raises(ValueError, "api.option('a').value.set('192.168.1.255')") + api.option('a').value.reset() + api.option('b').value.reset() + api.option('a').value.set('192.168.1.255') + raises(ValueError, "api.option('b').value.set('255.255.255.0')") + + +def test_consistency_ip_netmask_invalid(): + a = IPOption('a', '') + b = NetmaskOption('b', '') + od = OptionDescription('od', '', [a, b]) + raises(ConfigError, "b.impl_add_consistency('ip_netmask')") def test_consistency_network_netmask(): @@ -431,6 +442,13 @@ def test_consistency_network_netmask(): raises(ValueError, "api.option('a').value.set('192.168.1.1')") +def test_consistency_network_netmask_invalid(): + a = NetworkOption('a', '') + b = NetmaskOption('b', '') + od = OptionDescription('od', '', [a, b]) + raises(ConfigError, "b.impl_add_consistency('network_netmask')") + + def test_consistency_ip_in_network(): a = NetworkOption('a', '') b = NetmaskOption('b', '') @@ -452,26 +470,13 @@ def test_consistency_ip_in_network(): assert len(w) == 1 -# needs 3 arguments, not 2 -#def test_consistency_ip_in_network_len_error(): -# a = NetworkOption('a', '') -# b = NetmaskOption('b', '') -# c = IPOption('c', '') -# od = OptionDescription('od', '', [a, b, c]) -# raises(ConfigError, "c.impl_add_consistency('in_network', a)") - - -# needs 2 arguments, not 3 -#def test_consistency_ip_netmask_network_error(): -# a = IPOption('a', '') -# b = NetworkOption('b', '') -# c = NetmaskOption('c', '') -# od = OptionDescription('od', '', [a, b, c]) -# c.impl_add_consistency('ip_netmask', a, b) -# api = Config(od) -# api.option('a').value.set('192.168.1.1') -# api.option('b').value.set('192.168.1.0') -# raises(ConfigError, "api.option('c').value.set('255.255.255.0')") +def test_consistency_ip_in_network_invalid(): + a = NetworkOption('a', '') + b = NetmaskOption('b', '') + c = IPOption('c', '') + d = IPOption('d', '') + od = OptionDescription('od', '', [a, b, c, d]) + raises(ConfigError, "c.impl_add_consistency('in_network', a)") def test_consistency_ip_netmask_error_multi(): diff --git a/test/test_option_owner.py b/test/test_option_owner.py index 2ae87ee..4dfefc5 100644 --- a/test/test_option_owner.py +++ b/test/test_option_owner.py @@ -112,6 +112,17 @@ def test_setowner_for_value(): assert api.option('dummy').owner.get() == owners.new2 +def test_setowner_forbidden(): + gcdummy = BoolOption('dummy', 'dummy', default=False) + descr = OptionDescription('tiramisu', '', [gcdummy]) + api = Config(descr) + assert api.option('dummy').value.get() is False + assert api.option('dummy').owner.get() == 'default' + raises(ValueError, "api.owner.set('default')") + api.option('dummy').value.set(False) + raises(ValueError, "api.option('dummy').owner.set('default')") + + def test_setowner_read_only(): gcdummy = BoolOption('dummy', 'dummy', default=False) descr = OptionDescription('tiramisu', '', [gcdummy]) diff --git a/tiramisu/option/broadcastoption.py b/tiramisu/option/broadcastoption.py index 524fcef..ac0b662 100644 --- a/tiramisu/option/broadcastoption.py +++ b/tiramisu/option/broadcastoption.py @@ -49,7 +49,8 @@ class BroadcastOption(Option): current_opt, opts, vals, - warnings_only): + warnings_only, + context): if len(vals) != 3: raise ConfigError(_('invalid len for vals')) if None in vals: diff --git a/tiramisu/option/choiceoption.py b/tiramisu/option/choiceoption.py index d879c9b..a41cb93 100644 --- a/tiramisu/option/choiceoption.py +++ b/tiramisu/option/choiceoption.py @@ -85,7 +85,6 @@ class ChoiceOption(Option): current_opt=undefined): if current_opt is undefined: current_opt = self - #FIXME cache? but in context... values = self._choice_values if isinstance(values, FunctionType): if option_bag is undefined: diff --git a/tiramisu/option/ipoption.py b/tiramisu/option/ipoption.py index 0299b5b..c2f4113 100644 --- a/tiramisu/option/ipoption.py +++ b/tiramisu/option/ipoption.py @@ -101,7 +101,10 @@ class IPOption(Option): current_opt, opts, vals, - warnings_only): + warnings_only, + context): + if len(vals) != 3 and context is undefined: + raise ConfigError(_('ip_network needs an IP, a network and a netmask')) if len(vals) != 3 or None in vals: return ip, network, netmask = vals @@ -117,4 +120,5 @@ class IPOption(Option): opts[2]._cons_ip_netmask(current_opt, (opts[2], opts[0]), (netmask, ip), - warnings_only) + warnings_only, + context) diff --git a/tiramisu/option/netmaskoption.py b/tiramisu/option/netmaskoption.py index fb3b123..7f78d72 100644 --- a/tiramisu/option/netmaskoption.py +++ b/tiramisu/option/netmaskoption.py @@ -50,9 +50,12 @@ class NetmaskOption(Option): current_opt, opts, vals, - warnings_only): + warnings_only, + context): #opts must be (netmask, network) options - if len(vals) != 2 or None in vals: + if context is undefined and len(vals) != 2: + raise ConfigError(_('network_netmask needs a network and a netmask')) + if None in vals or len(vals) != 2: return return self.__cons_netmask(current_opt, opts, @@ -66,9 +69,12 @@ class NetmaskOption(Option): current_opt, opts, vals, - warnings_only): + warnings_only, + context): #opts must be (netmask, ip) options - if len(vals) != 2 or None in vals: + if context is undefined and len(vals) != 2: + raise ConfigError(_('ip_netmask needs an IP and a netmask')) + if None in vals or len(vals) != 2: return self.__cons_netmask(current_opt, opts, @@ -86,8 +92,6 @@ class NetmaskOption(Option): make_net, warnings_only, typ): - if len(opts) != 2: - raise ConfigError(_('invalid len for opts')) msg = None try: ip = IP('{0}/{1}'.format(val_ipnetwork, val_netmask), make_net=make_net) @@ -99,7 +103,7 @@ class NetmaskOption(Option): msg = _('this is a network with netmask "{0}" ("{1}")') else: msg = _('this is a network with {2} "{0}" ("{1}")') - if ip.broadcast() == val_ip: + elif ip.broadcast() == val_ip: if current_opt == opts[1]: msg = _('this is a broadcast with netmask "{0}" ("{1}")') else: diff --git a/tiramisu/option/option.py b/tiramisu/option/option.py index 04a990d..1592ca8 100644 --- a/tiramisu/option/option.py +++ b/tiramisu/option/option.py @@ -207,8 +207,9 @@ class Option(OnlyOption): return def _is_not_unique(value): - #FIXME pourquoi la longueur doit etre egal ? - if check_error and self.impl_is_unique() and len(set(value)) != len(value): + # if set(value) has not same length than value + if check_error and self.impl_is_unique() and \ + len(set(value)) != len(value): for idx, val in enumerate(value): if val in value[idx+1:]: raise ValueError(_('invalid value "{}", this value is already in "{}"' @@ -243,7 +244,6 @@ class Option(OnlyOption): if isinstance(_value, list): # pragma: no cover raise ValueError(_('which must not be a list').format(_value, self.impl_get_display_name())) - #FIXME a revoir ... if _value is not None: if check_error: # option validation @@ -271,8 +271,6 @@ class Option(OnlyOption): self.__class__.__name__, 0) else: raise err - #if is_multi is None: - # is_multi = self.impl_is_multi() try: val = value if not self.impl_is_multi(): @@ -332,8 +330,6 @@ class Option(OnlyOption): return False def impl_is_master_slaves(self, type_='both'): - """FIXME - """ master_slaves = self.impl_get_master_slaves() if master_slaves is not None: if type_ in ('both', 'master') and \ @@ -654,7 +650,8 @@ class Option(OnlyOption): getattr(self, func)(current_opt, all_cons_opts, values, - warnings_only) + warnings_only, + context) except ValueError as err: if warnings_only: msg = _('attention, "{0}" could be an invalid {1} for "{2}", {3}' @@ -672,7 +669,8 @@ class Option(OnlyOption): current_opt, opts, vals, - warnings_only): + warnings_only, + context): equal = [] is_current = False for idx_inf, val_inf in enumerate(vals): diff --git a/tiramisu/option/optiondescription.py b/tiramisu/option/optiondescription.py index c9032ce..5a25e47 100644 --- a/tiramisu/option/optiondescription.py +++ b/tiramisu/option/optiondescription.py @@ -74,6 +74,15 @@ class CacheOptionDescription(BaseOption): if not option.impl_is_symlinkoption(): properties = option.impl_getproperties() if 'force_store_value' in properties: + if option.impl_is_master_slaves('slave'): + # problem with index + raise ConfigError(_('the slave "{0}" cannot have ' + '"force_store_value" property').format( + option.impl_get_display_name())) + if option.issubdyn(): + raise ConfigError(_('the dynoption "{0}" cannot have ' + '"force_store_value" property').format( + option.impl_get_display_name())) force_store_values.append((subpath, option)) if 'force_default_on_freeze' in properties and \ 'frozen' not in properties and \ @@ -163,15 +172,6 @@ class CacheOptionDescription(BaseOption): value_setted = False values = context.cfgimpl_get_values() for subpath, option in self._cache_force_store_values: - if option.impl_is_master_slaves('slave'): - # problem with index - raise ConfigError(_('the slave "{0}" cannot have ' - '"force_store_value" property').format( - option.impl_get_display_name())) - if option.issubdyn(): - raise ConfigError(_('the dynoption "{0}" cannot have ' - '"force_store_value" property').format( - option.impl_get_display_name())) if not values._p_.hasvalue(subpath): config_bag = ConfigBag(context=context) option_bag = OptionBag() @@ -429,12 +429,6 @@ class OptionDescription(OptionDescriptionWalk): def impl_getdoc(self): return self.impl_get_information('doc') - def impl_validate(self, - *args, - **kwargs): - """usefull for OptionDescription""" - pass - # ____________________________________________________________ def impl_set_group_type(self, group_type): @@ -444,8 +438,8 @@ class OptionDescription(OptionDescriptionWalk): that lives in `setting.groups` """ if self._group_type != groups.default: - raise TypeError(_('cannot change group_type if already set ' - '(old {0}, new {1})').format(self._group_type, + raise ValueError(_('cannot change group_type if already set ' + '(old {0}, new {1})').format(self._group_type, group_type)) if not isinstance(group_type, groups.GroupType): raise ValueError(_('group_type: {0}' diff --git a/tiramisu/setting.py b/tiramisu/setting.py index d7ecce4..af2d3b0 100644 --- a/tiramisu/setting.py +++ b/tiramisu/setting.py @@ -785,6 +785,9 @@ class Settings(object): def setowner(self, owner): ":param owner: sets the default value for owner at the Config level" + if owner in forbidden_owners: + raise ValueError(_('set owner "{0}" is forbidden').format(str(owner))) + self._owner = owner def getowner(self): diff --git a/tiramisu/storage/util.py b/tiramisu/storage/util.py index 26fee32..49d6977 100644 --- a/tiramisu/storage/util.py +++ b/tiramisu/storage/util.py @@ -19,7 +19,7 @@ from time import time from .cache.dictionary import Cache as DictCache -def _display_classname(obj): +def _display_classname(obj): # pragma: no cover return(obj.__class__.__name__.lower()) DEBUG = False @@ -38,12 +38,12 @@ class Cache(DictCache): if slave, add index """ if 'cache' in props or 'cache' in self_props: - if DEBUG: + if DEBUG: # pragma: no cover print('setcache {} with index {} and value {} in {} ({})'.format(path, index, val, _display_classname(self), id(self))) self._setcache(path, index, val, time()) - elif DEBUG: + elif DEBUG: # pragma: no cover print('not setcache {} with index {} and value {} and props {} and {} in {} ({})'.format(path, index, val, @@ -60,7 +60,7 @@ class Cache(DictCache): self_props, type_): no_cache = False, None - if 'cache' in props: + if 'cache' in props or type_ == 'context_props': indexed = self._getcache(path, index) if indexed is None: return no_cache @@ -79,22 +79,22 @@ class Cache(DictCache): 'expire' in self_props): ntime = int(time()) if timestamp + expires_time >= ntime: - if DEBUG: + if DEBUG: # pragma: no cover print('getcache in cache (1)', path, value, _display_classname(self), id(self), index) return True, value else: - if DEBUG: + if DEBUG: # pragma: no cover print('getcache expired value for path {} < {}'.format( timestamp + expires_time, ntime)) # if expired, remove from cache #self.delcache(path) else: - if DEBUG: + if DEBUG: # pragma: no cover print('getcache in cache (2)', path, value, _display_classname(self), id(self), index) return True, value - if DEBUG: + if DEBUG: # pragma: no cover print('getcache {} with index {} not in {} cache'.format(path, index, _display_classname(self))) return no_cache @@ -102,14 +102,14 @@ class Cache(DictCache): def delcache(self, path): """remove cache for a specified path """ - if DEBUG: + if DEBUG: # pragma: no cover print('delcache', path, _display_classname(self), id(self)) if path in self._cache: self._delcache(path) def reset_all_cache(self): "empty the cache" - if DEBUG: + if DEBUG: # pragma: no cover print('reset_all_cache', _display_classname(self), id(self)) self._reset_all_cache() @@ -118,6 +118,6 @@ class Cache(DictCache): please only use it in test purpose example: {'path1': {'index1': ('value1', 'time1')}, 'path2': {'index2': ('value2', 'time2', )}} """ - if DEBUG: + if DEBUG: # pragma: no cover print('get_chached {} for {} ({})'.format(self._cache, _display_classname(self), id(self))) return self._get_cached() diff --git a/tiramisu/value.py b/tiramisu/value.py index ad0c8a3..7451e61 100644 --- a/tiramisu/value.py +++ b/tiramisu/value.py @@ -109,19 +109,11 @@ class Values(object): owners.default, index=_index, with_value=True) - - if owner != owners.default: + if owner != owners.default and not ('frozen' in option_bag.properties and \ + 'force_default_on_freeze' in option_bag.properties): # if a value is store in storage, check if not frozen + force_default_on_freeze # if frozen + force_default_on_freeze => force default value - if not ('frozen' in option_bag.properties and \ - 'force_default_on_freeze' in option_bag.properties): - if index is not None and not is_slave: - if len(value) > index: - return value[index] - #value is smaller than expected - #so return default value - else: - return value + return value return self.getdefaultvalue(option_bag) def getdefaultvalue(self, @@ -415,7 +407,7 @@ class Values(object): raise ConfigError(_("can't set owner for the symlinkoption \"{}\"" "").format(opt.impl_get_display_name())) if owner in forbidden_owners: - raise ConfigError(_('set owner "{0}" is forbidden').format(str(owner))) + raise ValueError(_('set owner "{0}" is forbidden').format(str(owner))) if not self._p_.hasvalue(option_bag.path): raise ConfigError(_('no value for {0} cannot change owner to {1}'