From b42b39742cf6dffe0d3d555e056d276c960b8ac3 Mon Sep 17 00:00:00 2001
From: Rafael Sarmiento <rafael.sarmiento@cscs.ch>
Date: Mon, 27 Jan 2020 14:57:31 +0100
Subject: [PATCH 1/7] change timer format

---
 reframe/core/fields.py           | 27 +++++++++++++++------------
 reframe/core/launchers/mpi.py    |  4 +++-
 reframe/core/pipeline.py         | 10 ++++------
 reframe/core/schedulers/local.py |  5 ++---
 reframe/core/schedulers/pbs.py   |  4 +++-
 reframe/core/schedulers/slurm.py |  4 +++-
 unittests/test_launchers.py      |  2 +-
 7 files changed, 31 insertions(+), 25 deletions(-)

diff --git a/reframe/core/fields.py b/reframe/core/fields.py
index 1a06980b73..f392a5b2aa 100644
--- a/reframe/core/fields.py
+++ b/reframe/core/fields.py
@@ -3,7 +3,9 @@
 #
 
 import copy
+import datetime
 import os
+import re
 
 import reframe.utility.typecheck as types
 from reframe.core.exceptions import user_deprecation_warning
@@ -100,23 +102,24 @@ def __set__(self, obj, value):
 
 
 class TimerField(TypedField):
-    '''Stores a timer in the form of a tuple ``(hh, mm, ss)``'''
+    '''Stores a timer in the form of a string ``'%dd%dh%dm%ds'``'''
 
     def __init__(self, fieldname, *other_types):
-        super().__init__(fieldname, types.Tuple[int, int, int], *other_types)
+        super().__init__(fieldname, datetime.timedelta, str, *other_types)
 
     def __set__(self, obj, value):
         self._check_type(value)
-        if value is not None:
-            # Check also the values for minutes and seconds
-            h, m, s = value
-            if h < 0 or m < 0 or s < 0:
-                raise ValueError('timer field must have '
-                                 'non-negative values')
-
-            if m > 59 or s > 59:
-                raise ValueError('minutes and seconds in a timer '
-                                 'field must not exceed 59')
+        if value and type(value) is not datetime.timedelta:
+            try:
+                time_dict = re.match(r'^((?P<days>\d+)d)*'
+                                     r'((?P<hours>\d+)h)*'
+                                     r'((?P<minutes>\d+)m)*'
+                                     r'((?P<seconds>\d+)s)*$',
+                                     value).groupdict()
+            except AttributeError:
+                raise Exception('invalid format')
+
+            value = datetime.timedelta(**{k:int(v) for k, v in time_dict.items() if v})
 
         # Call Field's __set__() method, type checking is already performed
         Field.__set__(self, obj, value)
diff --git a/reframe/core/launchers/mpi.py b/reframe/core/launchers/mpi.py
index 87436243c7..434c94a373 100644
--- a/reframe/core/launchers/mpi.py
+++ b/reframe/core/launchers/mpi.py
@@ -52,7 +52,9 @@ def command(self, job):
             ret += ['--job-name=%s' % job.name]
 
         if job.time_limit:
-            ret += ['--time=%d:%d:%d' % job.time_limit]
+            m, s = divmod(job.time_limit.total_seconds(), 60)
+            h, m = divmod(m, 60)
+            ret += ['--time=%d:%d:%d' % (h, m, s)]
 
         if job.stdout:
             ret += ['--output=%s' % job.stdout]
diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py
index 60f107e44e..1ae716a509 100644
--- a/reframe/core/pipeline.py
+++ b/reframe/core/pipeline.py
@@ -540,14 +540,12 @@ class RegressionTest(metaclass=RegressionTestMeta):
 
     #: Time limit for this test.
     #:
-    #: Time limit is specified as a three-tuple in the form ``(hh, mm, ss)``,
-    #: with ``hh >= 0``, ``0 <= mm <= 59`` and ``0 <= ss <= 59``.
+    #: Time limit is specified as a string in the form ``'%dd%dh%dm%ds'``,
     #: If set to :class:`None`, no time limit will be set.
     #: The default time limit of the system partition's scheduler will be used.
     #:
-    #:
-    #: :type: :class:`tuple[int]`
-    #: :default: ``(0, 10, 0)``
+    #: :type: :class:`str` or :class:`datetime.timedelta`
+    #: :default: ``'10m'``
     #:
     #: .. note::
     #:    .. versionchanged:: 2.15
@@ -710,7 +708,7 @@ def _rfm_init(self, name=None, prefix=None):
         self.variables = {}
 
         # Time limit for the check
-        self.time_limit = (0, 10, 0)
+        self.time_limit = '10m'
 
         # Runtime information of the test
         self._current_partition = None
diff --git a/reframe/core/schedulers/local.py b/reframe/core/schedulers/local.py
index cf0eabe2e3..d07e41344c 100644
--- a/reframe/core/schedulers/local.py
+++ b/reframe/core/schedulers/local.py
@@ -120,7 +120,7 @@ def cancel(self, job):
 
         # Set the time limit to the grace period and let wait() do the final
         # killing
-        job.time_limit = (0, 0, self._cancel_grace_period)
+        job.time_limit = datetime.timedelta(seconds=self._cancel_grace_period)
         self.wait(job)
 
     def wait(self, job):
@@ -138,8 +138,7 @@ def wait(self, job):
 
         # Convert job's time_limit to seconds
         if job.time_limit is not None:
-            h, m, s = job.time_limit
-            timeout = h * 3600 + m * 60 + s
+            timeout = job.time_limit.total_seconds()
         else:
             timeout = 0
 
diff --git a/reframe/core/schedulers/pbs.py b/reframe/core/schedulers/pbs.py
index 418e3a4bad..de302a3a1b 100644
--- a/reframe/core/schedulers/pbs.py
+++ b/reframe/core/schedulers/pbs.py
@@ -74,8 +74,10 @@ def emit_preamble(self, job):
         ]
 
         if job.time_limit is not None:
+            m, s = divmod(job.time_limit.total_seconds(), 60)
+            h, m = divmod(m, 60)
             preamble.append(
-                self._format_option('-l walltime=%d:%d:%d' % job.time_limit))
+                self._format_option('-l walltime=%d:%d:%d' % (h, m, s)))
 
         if job.sched_partition:
             preamble.append(
diff --git a/reframe/core/schedulers/slurm.py b/reframe/core/schedulers/slurm.py
index 769048c393..38a7c40fbf 100644
--- a/reframe/core/schedulers/slurm.py
+++ b/reframe/core/schedulers/slurm.py
@@ -154,8 +154,10 @@ def emit_preamble(self, job):
                      self._format_option(job.stderr, errfile_fmt)]
 
         if job.time_limit is not None:
+            m, s = divmod(job.time_limit.total_seconds(), 60)
+            h, m = divmod(m, 60)
             preamble.append(
-                self._format_option('%d:%d:%d' % job.time_limit, '--time={0}')
+                self._format_option('%d:%d:%d' % (h, m, s), '--time={0}')
             )
 
         if job.sched_exclusive_access:
diff --git a/unittests/test_launchers.py b/unittests/test_launchers.py
index a1df4e57eb..3e6a417add 100644
--- a/unittests/test_launchers.py
+++ b/unittests/test_launchers.py
@@ -56,7 +56,7 @@ def setUp(self):
         self.job.num_tasks_per_socket = 1
         self.job.num_cpus_per_task = 2
         self.job.use_smt = True
-        self.job.time_limit = (0, 10, 0)
+        self.job.time_limit = '10m'
         self.job.options += ['--gres=gpu:4', '#DW jobdw anything']
         self.job.launcher.options = ['--foo']
         self.minimal_job = Job.create(FakeJobScheduler(),

From f5dff2e320696b0df38730ceff315987ecd92a7d Mon Sep 17 00:00:00 2001
From: Rafael Sarmiento <rafael.sarmiento@cscs.ch>
Date: Mon, 27 Jan 2020 16:01:44 +0100
Subject: [PATCH 2/7] addapt unit test

---
 reframe/core/fields.py           | 13 +++++++------
 reframe/core/schedulers/local.py |  4 ++--
 unittests/test_fields.py         | 26 ++++++++++++--------------
 unittests/test_schedulers.py     |  6 +++---
 4 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/reframe/core/fields.py b/reframe/core/fields.py
index f392a5b2aa..bb62323122 100644
--- a/reframe/core/fields.py
+++ b/reframe/core/fields.py
@@ -111,15 +111,16 @@ def __set__(self, obj, value):
         self._check_type(value)
         if value and type(value) is not datetime.timedelta:
             try:
-                time_dict = re.match(r'^((?P<days>\d+)d)*'
-                                     r'((?P<hours>\d+)h)*'
-                                     r'((?P<minutes>\d+)m)*'
-                                     r'((?P<seconds>\d+)s)*$',
+                time_dict = re.match(r'^((?P<days>\d+)d)?'
+                                     r'((?P<hours>\d+)h)?'
+                                     r'((?P<minutes>\d+)m)?'
+                                     r'((?P<seconds>\d+)s)?$',
                                      value).groupdict()
             except AttributeError:
-                raise Exception('invalid format')
+                raise ValueError('invalid format for timer field')
 
-            value = datetime.timedelta(**{k:int(v) for k, v in time_dict.items() if v})
+            value = datetime.timedelta(**{k: int(v)
+                                          for k, v in time_dict.items() if v})
 
         # Call Field's __set__() method, type checking is already performed
         Field.__set__(self, obj, value)
diff --git a/reframe/core/schedulers/local.py b/reframe/core/schedulers/local.py
index d07e41344c..7e0dc6ae74 100644
--- a/reframe/core/schedulers/local.py
+++ b/reframe/core/schedulers/local.py
@@ -4,7 +4,7 @@
 import stat
 import subprocess
 import time
-from datetime import datetime
+from datetime import datetime, timedelta
 
 import reframe.core.schedulers as sched
 import reframe.utility.os_ext as os_ext
@@ -120,7 +120,7 @@ def cancel(self, job):
 
         # Set the time limit to the grace period and let wait() do the final
         # killing
-        job.time_limit = datetime.timedelta(seconds=self._cancel_grace_period)
+        job.time_limit = timedelta(seconds=self._cancel_grace_period)
         self.wait(job)
 
     def wait(self, job):
diff --git a/unittests/test_fields.py b/unittests/test_fields.py
index de6f0d4bab..2e1cd587f3 100644
--- a/unittests/test_fields.py
+++ b/unittests/test_fields.py
@@ -1,3 +1,4 @@
+import datetime
 import os
 import unittest
 
@@ -82,30 +83,27 @@ class FieldTester:
                 'field_maybe_none', type(None))
 
         tester = FieldTester()
-        tester.field = (65, 22, 47)
+        tester.field = '1d65h22m87s'
         tester.field_maybe_none = None
 
         self.assertIsInstance(FieldTester.field, fields.TimerField)
-        self.assertEqual((65, 22, 47), tester.field)
-        self.assertRaises(TypeError, exec, 'tester.field = (2,)',
+        self.assertEqual(datetime.timedelta(days=1, hours=65, minutes=22,
+                                            seconds=87), tester.field)
+        self.assertRaises(ValueError, exec, 'tester.field = "1e"',
                           globals(), locals())
-        self.assertRaises(TypeError, exec, 'tester.field = (2, 2)',
+        self.assertRaises(ValueError, exec, 'tester.field = "-10m5s"',
                           globals(), locals())
-        self.assertRaises(TypeError, exec, 'tester.field = (2, 2, 3.4)',
+        self.assertRaises(ValueError, exec, 'tester.field = "m10s"',
                           globals(), locals())
-        self.assertRaises(TypeError, exec, "tester.field = ('foo', 2, 3)",
+        self.assertRaises(ValueError, exec, 'tester.field = "10m10"',
                           globals(), locals())
-        self.assertRaises(TypeError, exec, 'tester.field = 3',
+        self.assertRaises(ValueError, exec, 'tester.field = "10m10m1s"',
                           globals(), locals())
-        self.assertRaises(ValueError, exec, 'tester.field = (-2, 3, 5)',
+        self.assertRaises(ValueError, exec, 'tester.field = "10m5s3m"',
                           globals(), locals())
-        self.assertRaises(ValueError, exec, 'tester.field = (100, -3, 4)',
+        self.assertRaises(ValueError, exec, 'tester.field = "10ms"',
                           globals(), locals())
-        self.assertRaises(ValueError, exec, 'tester.field = (100, 3, -4)',
-                          globals(), locals())
-        self.assertRaises(ValueError, exec, 'tester.field = (100, 65, 4)',
-                          globals(), locals())
-        self.assertRaises(ValueError, exec, 'tester.field = (100, 3, 65)',
+        self.assertRaises(TypeError, exec, 'tester.field = 10',
                           globals(), locals())
 
     def test_proxy_field(self):
diff --git a/unittests/test_schedulers.py b/unittests/test_schedulers.py
index 47b2508270..8736c03027 100644
--- a/unittests/test_schedulers.py
+++ b/unittests/test_schedulers.py
@@ -95,7 +95,7 @@ def assertScriptSanity(self, script_file):
 
     def setup_job(self):
         # Mock up a job submission
-        self.testjob.time_limit = (0, 5, 0)
+        self.testjob.time_limit = '5m'
         self.testjob.num_tasks = 16
         self.testjob.num_tasks_per_node = 2
         self.testjob.num_tasks_per_core = 1
@@ -129,7 +129,7 @@ def test_submit(self):
     def test_submit_timelimit(self, check_elapsed_time=True):
         self.setup_user()
         self.parallel_cmd = 'sleep 10'
-        self.testjob.time_limit = (0, 0, 2)
+        self.testjob.time_limit = '2s'
         self.prepare()
         t_job = datetime.now()
         self.testjob.submit()
@@ -238,7 +238,7 @@ def test_cancel_with_grace(self):
         self.parallel_cmd = 'sleep 5 &'
         self.pre_run = ['trap -- "" TERM']
         self.post_run = ['echo $!', 'wait']
-        self.testjob.time_limit = (0, 1, 0)
+        self.testjob.time_limit = '1m'
         self.testjob.scheduler._cancel_grace_period = 2
 
         self.prepare()

From fa4509388d93013cf6b75c49fe158f15174ba457 Mon Sep 17 00:00:00 2001
From: Rafael Sarmiento <rafael.sarmiento@cscs.ch>
Date: Mon, 27 Jan 2020 16:17:30 +0100
Subject: [PATCH 3/7] add unit test

---
 reframe/core/fields.py   | 2 +-
 unittests/test_fields.py | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/reframe/core/fields.py b/reframe/core/fields.py
index bb62323122..d19b110d20 100644
--- a/reframe/core/fields.py
+++ b/reframe/core/fields.py
@@ -109,7 +109,7 @@ def __init__(self, fieldname, *other_types):
 
     def __set__(self, obj, value):
         self._check_type(value)
-        if value and type(value) is not datetime.timedelta:
+        if value is not None and type(value) is not datetime.timedelta:
             try:
                 time_dict = re.match(r'^((?P<days>\d+)d)?'
                                      r'((?P<hours>\d+)h)?'
diff --git a/unittests/test_fields.py b/unittests/test_fields.py
index 2e1cd587f3..5a8f47d816 100644
--- a/unittests/test_fields.py
+++ b/unittests/test_fields.py
@@ -89,6 +89,9 @@ class FieldTester:
         self.assertIsInstance(FieldTester.field, fields.TimerField)
         self.assertEqual(datetime.timedelta(days=1, hours=65, minutes=22,
                                             seconds=87), tester.field)
+        tester.field = ''
+        self.assertEqual(datetime.timedelta(days=0, hours=0, minutes=0,
+                                            seconds=0), tester.field)
         self.assertRaises(ValueError, exec, 'tester.field = "1e"',
                           globals(), locals())
         self.assertRaises(ValueError, exec, 'tester.field = "-10m5s"',

From 92a8dd394bbc16f996ae36e1c1beadd8b38916de Mon Sep 17 00:00:00 2001
From: Rafael Sarmiento <rafael.sarmiento@cscs.ch>
Date: Fri, 14 Feb 2020 11:34:16 +0100
Subject: [PATCH 4/7] fix comments

---
 docs/advanced.rst                      |  2 +-
 docs/tutorial.rst                      |  2 +-
 reframe/core/fields.py                 | 31 ++++++++++++---------
 reframe/core/launchers/mpi.py          |  4 +--
 reframe/core/pipeline.py               |  4 +++
 reframe/core/schedulers/pbs.py         |  4 +--
 reframe/core/schedulers/slurm.py       |  4 +--
 reframe/utility/timer.py               |  5 ++++
 tutorial/advanced/advanced_example5.py |  2 +-
 unittests/test_fields.py               | 37 +++++++++++---------------
 10 files changed, 53 insertions(+), 42 deletions(-)
 create mode 100644 reframe/utility/timer.py

diff --git a/docs/advanced.rst b/docs/advanced.rst
index 1dd15d089d..7ec8b92933 100644
--- a/docs/advanced.rst
+++ b/docs/advanced.rst
@@ -227,7 +227,7 @@ The important bit here is the following line that sets the time limit for the te
   :lines: 12
   :dedent: 8
 
-The :attr:`time_limit <reframe.core.pipeline.RegressionTest.time_limit>` attribute is a three-tuple in the form ``(HOURS, MINUTES, SECONDS)``.
+The :attr:`time_limit <reframe.core.pipeline.RegressionTest.time_limit>` attribute is a string in the form ``<DAYS>d<HOURS>h<MINUTES>m<SECONDS>s)``.
 Time limits are implemented for all the scheduler backends.
 
 The sanity condition for this test verifies that associated job has been canceled due to the time limit (note that this message is SLURM-specific).
diff --git a/docs/tutorial.rst b/docs/tutorial.rst
index daac4aa035..73d9ff6432 100644
--- a/docs/tutorial.rst
+++ b/docs/tutorial.rst
@@ -527,7 +527,7 @@ For schedulers that do not provide the same functionality, some of the variables
 ================================================ ===========================================
       :class:`RegressionTest` attribute                    Corresponding SLURM option
 ================================================ ===========================================
-      ``time_limit = (0, 10, 30)``                         ``--time=00:10:30``
+      ``time_limit = '10m30s``                             ``--time=00:10:30``
       ``use_multithreading = True``                        ``--hint=multithread``
       ``use_multithreading = False``                       ``--hint=nomultithread``
       ``exclusive_access = True``                          ``--exclusive``
diff --git a/reframe/core/fields.py b/reframe/core/fields.py
index d19b110d20..d7afa6f692 100644
--- a/reframe/core/fields.py
+++ b/reframe/core/fields.py
@@ -102,25 +102,32 @@ def __set__(self, obj, value):
 
 
 class TimerField(TypedField):
-    '''Stores a timer in the form of a string ``'%dd%dh%dm%ds'``'''
+    '''Stores a timer in the form of a :class:`datetime.timedelta` object'''
 
     def __init__(self, fieldname, *other_types):
-        super().__init__(fieldname, datetime.timedelta, str, *other_types)
+        super().__init__(fieldname, datetime.timedelta, str,
+                         types.Tuple[int, int, int], *other_types)
 
     def __set__(self, obj, value):
         self._check_type(value)
-        if value is not None and type(value) is not datetime.timedelta:
-            try:
-                time_dict = re.match(r'^((?P<days>\d+)d)?'
-                                     r'((?P<hours>\d+)h)?'
-                                     r'((?P<minutes>\d+)m)?'
-                                     r'((?P<seconds>\d+)s)?$',
-                                     value).groupdict()
-            except AttributeError:
+        if isinstance(value, tuple):
+            user_deprecation_warning(
+                'setting a timer field as tuple is deprecated: '
+                'please use a string <days>d<hours>h<minutes>m<seconds>s')
+            h, m, s = value
+            value = datetime.timedelta(hours=h, minutes=m, seconds=s)
+
+        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_dict.items() if v})
+            value = datetime.timedelta(
+                **{k: int(v) for k, v in time_match.groupdict().items() if v})
 
         # Call Field's __set__() method, type checking is already performed
         Field.__set__(self, obj, value)
diff --git a/reframe/core/launchers/mpi.py b/reframe/core/launchers/mpi.py
index 434c94a373..589d98c73c 100644
--- a/reframe/core/launchers/mpi.py
+++ b/reframe/core/launchers/mpi.py
@@ -1,5 +1,6 @@
 from reframe.core.launchers import JobLauncher
 from reframe.core.launchers.registry import register_launcher
+from reframe.utility.timer import seconds_to_hms
 
 
 @register_launcher('srun')
@@ -52,8 +53,7 @@ def command(self, job):
             ret += ['--job-name=%s' % job.name]
 
         if job.time_limit:
-            m, s = divmod(job.time_limit.total_seconds(), 60)
-            h, m = divmod(m, 60)
+            h, m, s = seconds_to_hms(job.time_limit.total_seconds())
             ret += ['--time=%d:%d:%d' % (h, m, s)]
 
         if job.stdout:
diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py
index 1ae716a509..c5981cc805 100644
--- a/reframe/core/pipeline.py
+++ b/reframe/core/pipeline.py
@@ -544,6 +544,10 @@ class RegressionTest(metaclass=RegressionTestMeta):
     #: If set to :class:`None`, no time limit will be set.
     #: The default time limit of the system partition's scheduler will be used.
     #:
+    #: The value is internaly kept as a :class:`datetime.timedelta` object.
+    #: For example '2h30m' is represented as
+    #: datetime.timedelta(hours=2, minutes=30)
+    #:
     #: :type: :class:`str` or :class:`datetime.timedelta`
     #: :default: ``'10m'``
     #:
diff --git a/reframe/core/schedulers/pbs.py b/reframe/core/schedulers/pbs.py
index de302a3a1b..5968638488 100644
--- a/reframe/core/schedulers/pbs.py
+++ b/reframe/core/schedulers/pbs.py
@@ -17,6 +17,7 @@
 from reframe.core.exceptions import SpawnedProcessError, JobError
 from reframe.core.logging import getlogger
 from reframe.core.schedulers.registry import register_scheduler
+from reframe.utility.timer import seconds_to_hms
 
 
 # Time to wait after a job is finished for its standard output/error to be
@@ -74,8 +75,7 @@ def emit_preamble(self, job):
         ]
 
         if job.time_limit is not None:
-            m, s = divmod(job.time_limit.total_seconds(), 60)
-            h, m = divmod(m, 60)
+            h, m, s = seconds_to_hms(job.time_limit.total_seconds())
             preamble.append(
                 self._format_option('-l walltime=%d:%d:%d' % (h, m, s)))
 
diff --git a/reframe/core/schedulers/slurm.py b/reframe/core/schedulers/slurm.py
index 38a7c40fbf..5e0118f700 100644
--- a/reframe/core/schedulers/slurm.py
+++ b/reframe/core/schedulers/slurm.py
@@ -15,6 +15,7 @@
                                      JobBlockedError, JobError)
 from reframe.core.logging import getlogger
 from reframe.core.schedulers.registry import register_scheduler
+from reframe.utility.timer import seconds_to_hms
 
 
 def slurm_state_completed(state):
@@ -154,8 +155,7 @@ def emit_preamble(self, job):
                      self._format_option(job.stderr, errfile_fmt)]
 
         if job.time_limit is not None:
-            m, s = divmod(job.time_limit.total_seconds(), 60)
-            h, m = divmod(m, 60)
+            h, m, s = seconds_to_hms(job.time_limit.total_seconds())
             preamble.append(
                 self._format_option('%d:%d:%d' % (h, m, s), '--time={0}')
             )
diff --git a/reframe/utility/timer.py b/reframe/utility/timer.py
new file mode 100644
index 0000000000..2af0400798
--- /dev/null
+++ b/reframe/utility/timer.py
@@ -0,0 +1,5 @@
+def seconds_to_hms(seconds):
+    '''Convert time in seconds to a tuple (hour, minutes, seconds)'''
+    m, s = divmod(seconds, 60)
+    h, m = divmod(m, 60)
+    return h, m, s
diff --git a/tutorial/advanced/advanced_example5.py b/tutorial/advanced/advanced_example5.py
index 7b84d30fd6..87c17aac85 100644
--- a/tutorial/advanced/advanced_example5.py
+++ b/tutorial/advanced/advanced_example5.py
@@ -9,7 +9,7 @@ def __init__(self):
                       'of a user-defined time limit')
         self.valid_systems = ['daint:gpu', 'daint:mc']
         self.valid_prog_environs = ['*']
-        self.time_limit = (0, 1, 0)
+        self.time_limit = '1m'
         self.executable = 'sleep'
         self.executable_opts = ['100']
         self.sanity_patterns = sn.assert_found(
diff --git a/unittests/test_fields.py b/unittests/test_fields.py
index 5a8f47d816..714157cfa3 100644
--- a/unittests/test_fields.py
+++ b/unittests/test_fields.py
@@ -2,6 +2,7 @@
 import os
 import unittest
 
+import pytest
 import reframe.core.fields as fields
 from reframe.utility import ScopedDict
 
@@ -86,28 +87,22 @@ class FieldTester:
         tester.field = '1d65h22m87s'
         tester.field_maybe_none = None
 
-        self.assertIsInstance(FieldTester.field, fields.TimerField)
-        self.assertEqual(datetime.timedelta(days=1, hours=65, minutes=22,
-                                            seconds=87), tester.field)
+        assert isinstance(FieldTester.field, fields.TimerField)
+        assert (datetime.timedelta(days=1, hours=65, minutes=22,
+                                   seconds=87) == tester.field)
         tester.field = ''
-        self.assertEqual(datetime.timedelta(days=0, hours=0, minutes=0,
-                                            seconds=0), tester.field)
-        self.assertRaises(ValueError, exec, 'tester.field = "1e"',
-                          globals(), locals())
-        self.assertRaises(ValueError, exec, 'tester.field = "-10m5s"',
-                          globals(), locals())
-        self.assertRaises(ValueError, exec, 'tester.field = "m10s"',
-                          globals(), locals())
-        self.assertRaises(ValueError, exec, 'tester.field = "10m10"',
-                          globals(), locals())
-        self.assertRaises(ValueError, exec, 'tester.field = "10m10m1s"',
-                          globals(), locals())
-        self.assertRaises(ValueError, exec, 'tester.field = "10m5s3m"',
-                          globals(), locals())
-        self.assertRaises(ValueError, exec, 'tester.field = "10ms"',
-                          globals(), locals())
-        self.assertRaises(TypeError, exec, 'tester.field = 10',
-                          globals(), locals())
+        assert (datetime.timedelta(days=0, hours=0, minutes=0,
+                                   seconds=0) == tester.field)
+        with pytest.raises(ValueError):
+            exec('tester.field = "1e"', globals(), locals())
+            exec('tester.field = "-10m5s"', globals(), locals())
+            exec('tester.field = "10m-5s"', globals(), locals())
+            exec('tester.field = "m10s"', globals(), locals())
+            exec('tester.field = "10m10"', globals(), locals())
+            exec('tester.field = "10m10m1s"', globals(), locals())
+            exec('tester.field = "10m5s3m"', globals(), locals())
+            exec('tester.field = "10ms"', globals(), locals())
+            exec('tester.field = 10', globals(), locals())
 
     def test_proxy_field(self):
         class Target:

From d17a1e9df6e1544e96faf3309ece34a293a43140 Mon Sep 17 00:00:00 2001
From: rafael <rafael.sarmiento@cscs.ch>
Date: Thu, 20 Feb 2020 14:47:02 +0100
Subject: [PATCH 5/7] fix comments

---
 reframe/core/launchers/mpi.py    |  2 +-
 reframe/core/pipeline.py         |  4 ++--
 reframe/core/schedulers/pbs.py   |  2 +-
 reframe/core/schedulers/slurm.py |  2 +-
 reframe/utility/__init__.py      |  8 +++++++
 reframe/utility/timer.py         |  5 -----
 unittests/test_fields.py         | 36 +++++++++++++++++++++++++-------
 7 files changed, 42 insertions(+), 17 deletions(-)
 delete mode 100644 reframe/utility/timer.py

diff --git a/reframe/core/launchers/mpi.py b/reframe/core/launchers/mpi.py
index 589d98c73c..ca289613c1 100644
--- a/reframe/core/launchers/mpi.py
+++ b/reframe/core/launchers/mpi.py
@@ -1,6 +1,6 @@
 from reframe.core.launchers import JobLauncher
 from reframe.core.launchers.registry import register_launcher
-from reframe.utility.timer import seconds_to_hms
+from reframe.utility import seconds_to_hms
 
 
 @register_launcher('srun')
diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py
index c5981cc805..90aec7cc7d 100644
--- a/reframe/core/pipeline.py
+++ b/reframe/core/pipeline.py
@@ -540,13 +540,13 @@ class RegressionTest(metaclass=RegressionTestMeta):
 
     #: Time limit for this test.
     #:
-    #: Time limit is specified as a string in the form ``'%dd%dh%dm%ds'``,
+    #: Time limit is specified as a string in the form ``'%dd%dh%dm%ds'``.
     #: If set to :class:`None`, no time limit will be set.
     #: The default time limit of the system partition's scheduler will be used.
     #:
     #: The value is internaly kept as a :class:`datetime.timedelta` object.
     #: For example '2h30m' is represented as
-    #: datetime.timedelta(hours=2, minutes=30)
+    #: `datetime.timedelta(hours=2, minutes=30)`
     #:
     #: :type: :class:`str` or :class:`datetime.timedelta`
     #: :default: ``'10m'``
diff --git a/reframe/core/schedulers/pbs.py b/reframe/core/schedulers/pbs.py
index 5968638488..e945b6a665 100644
--- a/reframe/core/schedulers/pbs.py
+++ b/reframe/core/schedulers/pbs.py
@@ -17,7 +17,7 @@
 from reframe.core.exceptions import SpawnedProcessError, JobError
 from reframe.core.logging import getlogger
 from reframe.core.schedulers.registry import register_scheduler
-from reframe.utility.timer import seconds_to_hms
+from reframe.utility import seconds_to_hms
 
 
 # Time to wait after a job is finished for its standard output/error to be
diff --git a/reframe/core/schedulers/slurm.py b/reframe/core/schedulers/slurm.py
index 5e0118f700..a6e349d723 100644
--- a/reframe/core/schedulers/slurm.py
+++ b/reframe/core/schedulers/slurm.py
@@ -15,7 +15,7 @@
                                      JobBlockedError, JobError)
 from reframe.core.logging import getlogger
 from reframe.core.schedulers.registry import register_scheduler
-from reframe.utility.timer import seconds_to_hms
+from reframe.utility import seconds_to_hms
 
 
 def slurm_state_completed(state):
diff --git a/reframe/utility/__init__.py b/reframe/utility/__init__.py
index 30acf3f144..edf9def0ea 100644
--- a/reframe/utility/__init__.py
+++ b/reframe/utility/__init__.py
@@ -11,6 +11,14 @@
 from collections import UserDict
 
 
+def seconds_to_hms(seconds):
+    '''Convert time in seconds to a tuple of ``(hour, minutes, seconds)``.'''
+
+    m, s = divmod(seconds, 60)
+    h, m = divmod(m, 60)
+    return h, m, s
+
+
 def _get_module_name(filename):
     barename, _ = os.path.splitext(filename)
     if os.path.basename(filename) == '__init__.py':
diff --git a/reframe/utility/timer.py b/reframe/utility/timer.py
deleted file mode 100644
index 2af0400798..0000000000
--- a/reframe/utility/timer.py
+++ /dev/null
@@ -1,5 +0,0 @@
-def seconds_to_hms(seconds):
-    '''Convert time in seconds to a tuple (hour, minutes, seconds)'''
-    m, s = divmod(seconds, 60)
-    h, m = divmod(m, 60)
-    return h, m, s
diff --git a/unittests/test_fields.py b/unittests/test_fields.py
index 714157cfa3..e260b0566b 100644
--- a/unittests/test_fields.py
+++ b/unittests/test_fields.py
@@ -4,6 +4,7 @@
 
 import pytest
 import reframe.core.fields as fields
+from reframe.core.exceptions import ReframeDeprecationWarning
 from reframe.utility import ScopedDict
 
 
@@ -88,21 +89,44 @@ class FieldTester:
         tester.field_maybe_none = None
 
         assert isinstance(FieldTester.field, fields.TimerField)
-        assert (datetime.timedelta(days=1, hours=65, minutes=22,
-                                   seconds=87) == tester.field)
+        assert (datetime.timedelta(days=1, hours=65,
+                                   minutes=22, seconds=87) == tester.field)
+        tester.field = datetime.timedelta(days=1, hours=65,
+                                          minutes=22, seconds=87)
+        assert (datetime.timedelta(days=1, hours=65,
+                                   minutes=22, seconds=87) == tester.field)
         tester.field = ''
-        assert (datetime.timedelta(days=0, hours=0, minutes=0,
-                                   seconds=0) == tester.field)
+        assert (datetime.timedelta(days=0, hours=0,
+                                   minutes=0, seconds=0) == tester.field)
+        with self.assertWarns(ReframeDeprecationWarning):
+            tester.field = (65, 22, 87)
+
         with pytest.raises(ValueError):
             exec('tester.field = "1e"', globals(), locals())
+
+        with pytest.raises(ValueError):
             exec('tester.field = "-10m5s"', globals(), locals())
+
+        with pytest.raises(ValueError):
             exec('tester.field = "10m-5s"', globals(), locals())
+
+        with pytest.raises(ValueError):
             exec('tester.field = "m10s"', globals(), locals())
+
+        with pytest.raises(ValueError):
             exec('tester.field = "10m10"', globals(), locals())
+
+        with pytest.raises(ValueError):
             exec('tester.field = "10m10m1s"', globals(), locals())
+
+        with pytest.raises(ValueError):
             exec('tester.field = "10m5s3m"', globals(), locals())
+
+        with pytest.raises(ValueError):
             exec('tester.field = "10ms"', globals(), locals())
-            exec('tester.field = 10', globals(), locals())
+
+        with pytest.raises(ValueError):
+            exec('tester.field = "10"', globals(), locals())
 
     def test_proxy_field(self):
         class Target:
@@ -127,8 +151,6 @@ class Proxy:
         self.assertEqual(4, t.b)
 
     def test_deprecated_field(self):
-        from reframe.core.exceptions import ReframeDeprecationWarning
-
         class FieldTester:
             value = fields.DeprecatedField(fields.TypedField('value', int),
                                            'value field is deprecated')

From 1f7c256436cb4095f16f762eb064d77cfe2707a3 Mon Sep 17 00:00:00 2001
From: rafael <rafael.sarmiento@cscs.ch>
Date: Fri, 21 Feb 2020 13:07:45 +0100
Subject: [PATCH 6/7] fix comments

---
 reframe/core/fields.py   | 2 +-
 reframe/core/pipeline.py | 8 +++++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/reframe/core/fields.py b/reframe/core/fields.py
index b187d99e0c..cf37b27e6a 100644
--- a/reframe/core/fields.py
+++ b/reframe/core/fields.py
@@ -118,7 +118,7 @@ def __set__(self, obj, value):
         if isinstance(value, tuple):
             user_deprecation_warning(
                 "setting a timer field from a tuple is deprecated: "
-                "please use a string <days>d<hours>h<minutes>m<seconds>s")
+                "please use a string '<days>d<hours>h<minutes>m<seconds>s'")
             h, m, s = value
             value = datetime.timedelta(hours=h, minutes=m, seconds=s)
 
diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py
index 48b41d3220..b1bd9c3809 100644
--- a/reframe/core/pipeline.py
+++ b/reframe/core/pipeline.py
@@ -557,10 +557,16 @@ class RegressionTest(metaclass=RegressionTestMeta):
     #: :default: ``'10m'``
     #:
     #: .. note::
-    #:    .. versionchanged:: 3.0
+    #:    .. versionchanged:: 2.15
     #:
     #:    This attribute may be set to :class:`None`.
     #:
+    #: .. note::
+    #:    .. versionchanged:: 3.0
+    #:
+    #:    The format to set this attribute has changed and the previous
+    #:    format has been deprecated.
+    #:
     time_limit = fields.TimerField('time_limit', type(None))
 
     #: Extra resources for this test.

From 1a576b135aea4d4a925e5eb965876ac21773a2ab Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Fri, 21 Feb 2020 13:27:24 +0100
Subject: [PATCH 7/7] Fix final PR comments

---
 reframe/core/pipeline.py | 8 ++++----
 unittests/test_fields.py | 7 ++++++-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py
index b1bd9c3809..afceb0ac60 100644
--- a/reframe/core/pipeline.py
+++ b/reframe/core/pipeline.py
@@ -545,7 +545,8 @@ class RegressionTest(metaclass=RegressionTestMeta):
 
     #: Time limit for this test.
     #:
-    #: Time limit is specified as a string in the form ``'%dd%dh%dm%ds'``.
+    #: Time limit is specified as a string in the form
+    #: ``<days>d<hours>h<minutes>m<seconds>s``.
     #: If set to :class:`None`, no time limit will be set.
     #: The default time limit of the system partition's scheduler will be used.
     #:
@@ -561,11 +562,10 @@ class RegressionTest(metaclass=RegressionTestMeta):
     #:
     #:    This attribute may be set to :class:`None`.
     #:
-    #: .. note::
+    #: .. warning::
     #:    .. versionchanged:: 3.0
     #:
-    #:    The format to set this attribute has changed and the previous
-    #:    format has been deprecated.
+    #:    The old syntax using a ``(h, m, s)`` tuple is deprecated.
     #:
     time_limit = fields.TimerField('time_limit', type(None))
 
diff --git a/unittests/test_fields.py b/unittests/test_fields.py
index e3d6f78dc6..6a980e7fb2 100644
--- a/unittests/test_fields.py
+++ b/unittests/test_fields.py
@@ -1,8 +1,13 @@
+# Copyright 2016-2020 Swiss National Supercomputing Centre (CSCS/ETH Zurich)
+# ReFrame Project Developers. See the top-level LICENSE file for details.
+#
+# SPDX-License-Identifier: BSD-3-Clause
+
 import datetime
 import os
+import pytest
 import unittest
 
-import pytest
 import reframe.core.fields as fields
 from reframe.core.exceptions import ReframeDeprecationWarning
 from reframe.utility import ScopedDict