From 8d27b50a7c4afa91afb2e51c1e46bf3f678dd920 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Wed, 9 Aug 2017 14:56:32 +0100 Subject: [PATCH 1/3] tests: add tests documenting enum behavior Add tests that exercising various ways of creating and using WA enums. --- tests/test_utils.py | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index 270beaa4..60dec0dc 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -18,7 +18,7 @@ from unittest import TestCase from nose.tools import raises, assert_equal, assert_not_equal, assert_in, assert_not_in -from nose.tools import assert_true, assert_false +from nose.tools import assert_true, assert_false, assert_raises, assert_is, assert_list_equal from wa.utils.types import (list_or_integer, list_or_bool, caseless_string, arguments, prioritylist, enum, level) @@ -97,6 +97,37 @@ class TestPriorityList(TestCase): class TestEnumLevel(TestCase): + def test_enum_creation(self): + e = enum(['one', 'two', 'three']) + assert_list_equal(e.values, [0, 1, 2]) + + e = enum(['one', 'two', 'three'], start=10) + assert_list_equal(e.values, [10, 11, 12]) + + e = enum(['one', 'two', 'three'], start=-10, step=10) + assert_list_equal(e.values, [-10, 0, 10]) + + def test_enum_behavior(self): + e = enum(['one', 'two', 'three']) + + # case-insensitive level name and level value may all + # be used for equality comparisons. + assert_equal(e.one, 'one') + assert_equal(e.one, 'ONE') + assert_equal(e.one, 0) + assert_not_equal(e.one, '0') + + # ditto for enum membership tests + assert_in('one', e.values) + assert_in(2, e.values) + assert_not_in('five', e.values) + + # The same level object returned, only when + # passing in a valid level name/value. + assert_is(e('one'), e('ONE')) + assert_is(e('one'), e(0)) + assert_raises(ValueError, e, 'five') + def test_serialize_level(self): l = level('test', 1) s = l.to_pod() From a9959550af6b16b5da4972ef41848940d8f6e4fb Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Wed, 9 Aug 2017 15:24:57 +0100 Subject: [PATCH 2/3] utils/types: better enum class member setting - What used to be enum.values is now enum.levels. - Add enum.names and enum.values that are lists of enum's levels' names and values respectively. - Add a check on creation to make sure that provided level names do not conflict with the atomatically created members. --- tests/test_utils.py | 14 +++++++++++--- wa/framework/configuration/core.py | 2 +- wa/framework/instrumentation.py | 2 +- wa/utils/types.py | 14 +++++++++++--- 4 files changed, 24 insertions(+), 8 deletions(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index 60dec0dc..c3469f9e 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -107,6 +107,14 @@ class TestEnumLevel(TestCase): e = enum(['one', 'two', 'three'], start=-10, step=10) assert_list_equal(e.values, [-10, 0, 10]) + def test_enum_name_conflicts(self): + assert_raises(ValueError, enum, ['names', 'one', 'two']) + + e = enum(['NAMES', 'one', 'two']) + assert_in('names', e.levels) + assert_list_equal(e.names, ['names', 'one', 'two']) + assert_equal(e.NAMES, 'names') + def test_enum_behavior(self): e = enum(['one', 'two', 'three']) @@ -118,9 +126,9 @@ class TestEnumLevel(TestCase): assert_not_equal(e.one, '0') # ditto for enum membership tests - assert_in('one', e.values) - assert_in(2, e.values) - assert_not_in('five', e.values) + assert_in('one', e.levels) + assert_in(2, e.levels) + assert_not_in('five', e.levels) # The same level object returned, only when # passing in a valid level name/value. diff --git a/wa/framework/configuration/core.py b/wa/framework/configuration/core.py index 3665436e..0e0bbf73 100644 --- a/wa/framework/configuration/core.py +++ b/wa/framework/configuration/core.py @@ -741,7 +741,7 @@ class RunConfiguration(Configuration): 'retry_on_status', kind=list_of(Status), default=['FAILED', 'PARTIAL'], - allowed_values=Status.values[Status.RUNNING.value:], + allowed_values=Status.levels[Status.RUNNING.value:], description=''' This is list of statuses on which a job will be considered to have failed and will be automatically retried up to ``max_retries`` diff --git a/wa/framework/instrumentation.py b/wa/framework/instrumentation.py index 3fe67de3..009a2446 100644 --- a/wa/framework/instrumentation.py +++ b/wa/framework/instrumentation.py @@ -186,7 +186,7 @@ def priority(priority): def wrapper(*args, **kwargs): return func(*args, **kwargs) wrapper.func_name = func.func_name - if priority in Priority.values: + if priority in Priority.levels: wrapper.priority = Priority(priority) else: if not isinstance(priority, int): diff --git a/wa/utils/types.py b/wa/utils/types.py index 98e2b079..2db72597 100644 --- a/wa/utils/types.py +++ b/wa/utils/types.py @@ -557,7 +557,7 @@ def enum(args, start=0, step=1): @classmethod def from_pod(cls, pod): lv = level.from_pod(pod) - for enum_level in cls.values: + for enum_level in cls.levels: if enum_level == lv: return enum_level msg = 'Unexpected value "{}" for enum.' @@ -574,16 +574,24 @@ def enum(args, start=0, step=1): raise ValueError('Invalid enum value: {}'.format(repr(name))) + reserved = ['values', 'levels', 'names'] + levels = [] n = start for v in args: - name = caseless_string(identifier(v)) + id_v = identifier(v) + if id_v in reserved: + message = 'Invalid enum level name "{}"; must not be in {}' + raise ValueError(message.format(v, reserved)) + name = caseless_string(id_v) lv = level(v, n) setattr(Enum, name, lv) levels.append(lv) n += step - setattr(Enum, 'values', levels) + setattr(Enum, 'levels', levels) + setattr(Enum, 'values', [lv.value for lv in levels]) + setattr(Enum, 'names', [lv.name for lv in levels]) return Enum From 3f87f3ab072041a727fc1c54c5e55a0ee4bdcbea Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Wed, 9 Aug 2017 15:32:26 +0100 Subject: [PATCH 3/3] utils/misc: remove enum_metaclass It is unused and has been obsoleted by enum type in utils/types. --- wa/utils/misc.py | 44 -------------------------------------------- 1 file changed, 44 deletions(-) diff --git a/wa/utils/misc.py b/wa/utils/misc.py index f299cf43..76303dd9 100644 --- a/wa/utils/misc.py +++ b/wa/utils/misc.py @@ -235,50 +235,6 @@ def get_pager(): return pager -def enum_metaclass(enum_param, return_name=False, start=0): - """ - Returns a ``type`` subclass that may be used as a metaclass for - an enum. - - Paremeters: - - :enum_param: the name of class attribute that defines enum values. - The metaclass will add a class attribute for each value in - ``enum_param``. The value of the attribute depends on the type - of ``enum_param`` and on the values of ``return_name``. If - ``return_name`` is ``True``, then the value of the new attribute is - the name of that attribute; otherwise, if ``enum_param`` is a ``list`` - or a ``tuple``, the value will be the index of that param in - ``enum_param``, optionally offset by ``start``, otherwise, it will - be assumed that ``enum_param`` implementa a dict-like inteface and - the value will be ``enum_param[attr_name]``. - :return_name: If ``True``, the enum values will the names of enum attributes. If - ``False``, the default, the values will depend on the type of - ``enum_param`` (see above). - :start: If ``enum_param`` is a list or a tuple, and ``return_name`` is ``False``, - this specifies an "offset" that will be added to the index of the attribute - within ``enum_param`` to form the value. - - - """ - class __EnumMeta(type): - def __new__(mcs, clsname, bases, attrs): - cls = type.__new__(mcs, clsname, bases, attrs) - values = getattr(cls, enum_param, []) - if return_name: - for name in values: - setattr(cls, name, name) - else: - if isinstance(values, list) or isinstance(values, tuple): - for i, name in enumerate(values): - setattr(cls, name, i + start) - else: # assume dict-like - for name in values: - setattr(cls, name, values[name]) - return cls - return __EnumMeta - - _bash_color_regex = re.compile('\x1b\[[0-9;]+m')