[bugfix] Allow proper assignment of time_limit from command line

reframe/core/fields.py

@@ -7,9 +7,6 @@
 # Useful descriptors for advanced operations on fields
 #
 
-import datetime
-import re
-
 import reframe.utility.typecheck as types
 from reframe.core.warnings import user_deprecation_warning
 from reframe.utility import ScopedDict
@@ -65,9 +62,11 @@ def __set__(self, obj, value):
 class TypedField(Field):
     '''Stores a field of predefined type'''
 
-    def __init__(self, main_type, *other_types, attr_name=None):
+    def __init__(self, main_type, *other_types,
+                 attr_name=None, allow_implicit=False):
         super().__init__(attr_name)
         self._types = (main_type,) + other_types
+        self._allow_implicit = allow_implicit
         if not all(isinstance(t, type) for t in self._types):
             raise TypeError('{0} is not a sequence of types'.
                             format(self._types))
@@ -88,8 +87,9 @@ def __set__(self, obj, value):
             self._check_type(value)
         except TypeError:
             raw_value = remove_convertible(value)
-            if raw_value is value:
-                # value was not convertible; reraise
+            if raw_value is value and not self._allow_implicit:
+                # value was not convertible and the field does not allow
+                # implicit conversions; re-raise
                 raise
 
             # Try to convert value to any of the supported types
@@ -137,35 +137,6 @@ def __set__(self, obj, value):
         raise ValueError('attempt to set a read-only variable')
 
 
-class TimerField(TypedField):
-    '''Stores a timer in the form of a :class:`datetime.timedelta` object'''
-
-    def __init__(self, *other_types, attr_name=None):
-        super().__init__(str, int, float, *other_types, attr_name=attr_name)
-
-    def __set__(self, obj, value):
-        value = remove_convertible(value)
-        self._check_type(value)
-        if isinstance(value, str):
-            time_match = re.match(r'^((?P<days>\d+)d)?'
-                                  r'((?P<hours>\d+)h)?'
-                                  r'((?P<minutes>\d+)m)?'
-                                  r'((?P<seconds>\d+)s)?$',
-                                  value)
-            if not time_match:
-                raise ValueError('invalid format for timer field')
-
-            value = datetime.timedelta(
-                **{k: int(v) for k, v in time_match.groupdict().items() if v}
-            ).total_seconds()
-        elif isinstance(value, float) or isinstance(value, int):
-            if value < 0:
-                raise ValueError('timer field value cannot be negative')
-
-        # Call Field's __set__() method, type checking is already performed
-        Field.__set__(self, obj, value)
-
-
 class ScopedDictField(TypedField):
     '''Stores a ScopedDict with a specific type.
 

reframe/core/pipeline.py

@@ -625,9 +625,8 @@ def pipeline_hooks(cls):
     #:
     #: :type: :class:`str` or :class:`datetime.timedelta`
     #: :default: :class:`None`
-    max_pending_time = variable(
-        type(None), field=fields.TimerField, value=None, loggable=True
-    )
+    max_pending_time = variable(type(None), typ.Duration, value=None,
+                                loggable=True, allow_implicit=True)
 
     #: Specify whether this test needs exclusive access to nodes.
     #:
@@ -860,8 +859,8 @@ def pipeline_hooks(cls):
     #:    .. versionchanged:: 3.5.1
     #:       The default value is now :class:`None` and it can be set globally
     #:       per partition via the configuration.
-    time_limit = variable(type(None), field=fields.TimerField,
-                          value=None, loggable=True)
+    time_limit = variable(type(None), typ.Duration, value=None,
+                          loggable=True, allow_implicit=True)
 
     #: .. versionadded:: 3.5.1
     #:
@@ -871,8 +870,8 @@ def pipeline_hooks(cls):
     #:
     #: :type: :class:`str` or :class:`float` or :class:`int`
     #: :default: :class:`None`
-    build_time_limit = variable(type(None), field=fields.TimerField,
-                                value=None, loggable=True)
+    build_time_limit = variable(type(None), typ.Duration, value=None,
+                                loggable=True, allow_implicit=True)
 
     #: .. versionadded:: 2.8
     #:

reframe/core/schedulers/__init__.py

@@ -10,7 +10,6 @@
 import abc
 import time
 
-import reframe.core.fields as fields
 import reframe.core.runtime as runtime
 import reframe.core.shell as shell
 import reframe.utility.jsonext as jsonext
@@ -261,7 +260,8 @@ class Job(jsonext.JSONSerializable, metaclass=JobMeta):
     #:    based on the test information.
     #:
     #: .. versionadded:: 3.11.0
-    time_limit = variable(type(None), field=fields.TimerField, value=None)
+    time_limit = variable(type(None), typ.Duration,
+                          value=None, allow_implicit=True)
 
     #: Maximum pending time for this job.
     #:
@@ -273,8 +273,8 @@ class Job(jsonext.JSONSerializable, metaclass=JobMeta):
     #:    based on the test information.
     #:
     #: .. versionadded:: 3.11.0
-    max_pending_time = variable(type(None),
-                                field=fields.TimerField, value=None)
+    max_pending_time = variable(type(None), typ.Duration,
+                                value=None, allow_implicit=True)
 
     #: Arbitrary options to be passed to the backend job scheduler.
     #:

reframe/utility/typecheck.py

@@ -94,6 +94,7 @@
 '''
 
 import abc
+import datetime
 import re
 
 
@@ -152,19 +153,6 @@ def __call__(cls, *args, **kwargs):
 # container types
 
 
-class _CompositeType(abc.ABCMeta):
-    def __instancecheck__(cls, inst):
-        assert hasattr(cls, '_types') and len(cls._types) == 2
-        return (issubclass(type(inst), cls._types[0]) or
-                issubclass(type(inst), cls._types[1]))
-
-
-class _InvertedType(abc.ABCMeta):
-    def __instancecheck__(cls, inst):
-        assert hasattr(cls, '_xtype')
-        return not issubclass(type(inst), cls._xtype)
-
-
 class _BuiltinType(ConvertibleType):
     def __init__(cls, name, bases, namespace):
         # Make sure that the class defines `_type`
@@ -175,8 +163,7 @@ def __init__(cls, name, bases, namespace):
 
     def __instancecheck__(cls, inst):
         if hasattr(cls, '_types'):
-            return (issubclass(type(inst), cls._types[0]) or
-                    issubclass(type(inst), cls._types[1]))
+            return any(issubclass(type(inst), t) for t in cls._types)
 
         if hasattr(cls, '_xtype'):
             return not issubclass(type(inst), cls._xtype)
@@ -424,3 +411,50 @@ def make_meta_type(name, cls, metacls=_BuiltinType):
 Set     = make_meta_type('Set', set, _SequenceType)
 Str     = make_meta_type('Str', str, _StrType)
 Tuple   = make_meta_type('Tuple', tuple, _TupleType)
+
+
+class Duration(metaclass=ConvertibleType):
+    '''A float type that represents duration in seconds.
+
+    This type supports the following implicit conversions:
+
+    - From integer values
+    - From string values in the form of ``<days>d<hours>h<minutes>m<seconds>s``
+
+    .. versionadded:: 4.2
+
+    '''
+
+    def _assert_non_negative(val):
+        if val < 0:
+            raise ValueError('duration cannot be negative')
+
+        return val
+
+    @classmethod
+    def __rfm_cast_int__(cls, val):
+        return Duration._assert_non_negative(float(val))
+
+    @classmethod
+    def __rfm_cast_float__(cls, val):
+        return Duration._assert_non_negative(val)
+
+    @classmethod
+    def __rfm_cast_str__(cls, val):
+        # First try to convert to a float
+        try:
+            val = float(val)
+        except ValueError:
+            time_match = re.match(r'^((?P<days>\d+)d)?'
+                                  r'((?P<hours>\d+)h)?'
+                                  r'((?P<minutes>\d+)m)?'
+                                  r'((?P<seconds>\d+)s)?$',
+                                  val)
+            if not time_match:
+                raise ValueError(f'invalid duration: {val}') from None
+
+            val = datetime.timedelta(
+                **{k: int(v) for k, v in time_match.groupdict().items() if v}
+            ).total_seconds()
+
+        return Duration._assert_non_negative(val)

unittests/test_fields.py

@@ -3,7 +3,6 @@
 #
 # SPDX-License-Identifier: BSD-3-Clause
 
-import datetime
 import pytest
 import semver
 import warnings
@@ -67,6 +66,7 @@ def __init__(self):
     class FieldTester:
         field = fields.TypedField(ClassA)
         field_any = fields.TypedField(ClassA, str, type(None))
+        field_convertible = fields.TypedField(int, allow_implicit=True)
 
         def __init__(self, value):
             self.field = value
@@ -88,6 +88,9 @@ def __init__(self, value):
     with pytest.raises(TypeError):
         tester.field_any = 3
 
+    tester.field_convertible = 1
+    tester.field_convertible = '1'
+
 
 def test_typed_field_convertible():
     class FieldTester:
@@ -105,53 +108,6 @@ class FieldTester:
         tester.fieldC = fields.make_convertible(None)
 
 
-def test_timer_field():
-    class FieldTester:
-        field = fields.TimerField()
-        field_maybe_none = fields.TimerField(type(None))
-
-    tester = FieldTester()
-    tester.field = '1d65h22m87s'
-    tester.field_maybe_none = None
-    assert isinstance(FieldTester.field, fields.TimerField)
-    secs = datetime.timedelta(days=1, hours=65,
-                              minutes=22, seconds=87).total_seconds()
-    assert tester.field == secs
-    tester.field = secs
-    assert tester.field == secs
-    tester.field = ''
-    assert tester.field == 0
-    with pytest.raises(ValueError):
-        tester.field = '1e'
-
-    with pytest.raises(ValueError):
-        tester.field = '-10m5s'
-
-    with pytest.raises(ValueError):
-        tester.field = '10m-5s'
-
-    with pytest.raises(ValueError):
-        tester.field = 'm10s'
-
-    with pytest.raises(ValueError):
-        tester.field = '10m10'
-
-    with pytest.raises(ValueError):
-        tester.field = '10m10m1s'
-
-    with pytest.raises(ValueError):
-        tester.field = '10m5s3m'
-
-    with pytest.raises(ValueError):
-        tester.field = '10ms'
-
-    with pytest.raises(ValueError):
-        tester.field = '10'
-
-    with pytest.raises(ValueError):
-        tester.field = -10
-
-
 def test_deprecated_field():
     class FieldTester:
         value = fields.DeprecatedField(fields.TypedField(int),

unittests/test_typecheck.py

@@ -40,6 +40,46 @@ def test_bool_type():
     assert typ.Bool('no') is False
 
 
+def test_duration_type():
+    assert typ.Duration(10) == 10
+    assert typ.Duration(10.5) == 10.5
+    assert typ.Duration('10') == 10
+    assert typ.Duration('10.5') == 10.5
+    assert typ.Duration('10s') == 10
+    assert typ.Duration('10m') == 600
+    assert typ.Duration('10m30s') == 630
+    assert typ.Duration('10h') == 36000
+    assert typ.Duration('1d') == 86400
+    assert typ.Duration('1d65h22m87s') == 321807
+
+    with pytest.raises(ValueError):
+        typ.Duration('1e')
+
+    with pytest.raises(ValueError):
+        typ.Duration('-10m5s')
+
+    with pytest.raises(ValueError):
+        typ.Duration('10m-5s')
+
+    with pytest.raises(ValueError):
+        typ.Duration('m10s')
+
+    with pytest.raises(ValueError):
+        typ.Duration('10m10')
+
+    with pytest.raises(ValueError):
+        typ.Duration('10m10m1s')
+
+    with pytest.raises(ValueError):
+        typ.Duration('10m5s3m')
+
+    with pytest.raises(ValueError):
+        typ.Duration('10ms')
+
+    with pytest.raises(ValueError):
+        typ.Duration(-10)
+
+
 def test_list_type():
     l = [1, 2]
     ll = [[1, 2], [3, 4]]