From 8d47856125a8031c691fd00312fb681cf92a7e82 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Thu, 7 Jun 2018 11:32:41 +0200 Subject: [PATCH 1/6] Add extra filter for empty prg environs * Create the corresponding unit test. --- reframe/frontend/cli.py | 5 +++-- unittests/test_cli.py | 15 ++++++++++++--- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/reframe/frontend/cli.py b/reframe/frontend/cli.py index 0e0a68b6d4..47007cbf78 100644 --- a/reframe/frontend/cli.py +++ b/reframe/frontend/cli.py @@ -351,8 +351,9 @@ def main(): # Filter checks by prgenv if not options.skip_prgenv_check: checks_matched = filter( - lambda c: c if all(c.supports_environ(e) - for e in options.prgenv) else None, + lambda c: c if (c.valid_prog_environs + and all(c.supports_environ(e) + for e in options.prgenv)) else None, checks_matched ) diff --git a/unittests/test_cli.py b/unittests/test_cli.py index 3708fad783..b355e7d000 100644 --- a/unittests/test_cli.py +++ b/unittests/test_cli.py @@ -257,19 +257,19 @@ def test_checkpath_recursion(self): self.checkpath = [] returncode, stdout, _ = self._run_reframe() num_checks_default = re.search( - 'Found (\d+) check', stdout, re.MULTILINE).group(1) + r'Found (\d+) check', stdout, re.MULTILINE).group(1) self.checkpath = ['checks/'] self.more_options = ['-R'] returncode, stdout, _ = self._run_reframe() num_checks_in_checkdir = re.search( - 'Found (\d+) check', stdout, re.MULTILINE).group(1) + r'Found (\d+) check', stdout, re.MULTILINE).group(1) self.assertEqual(num_checks_in_checkdir, num_checks_default) self.more_options = [] returncode, stdout, stderr = self._run_reframe() num_checks_in_checkdir = re.search( - 'Found (\d+) check', stdout, re.MULTILINE).group(1) + r'Found (\d+) check', stdout, re.MULTILINE).group(1) self.assertEqual('0', num_checks_in_checkdir) def test_same_output_stage_dir(self): @@ -317,3 +317,12 @@ def test_timestamp_option(self): returncode, stdout, _ = self._run_reframe() self.assertNotEqual(0, returncode) self.assertIn(timefmt, stdout) + + def test_list_check_with_empty_prgenvs(self): + self.checkpath = ['unittests/resources/checks/frontend_checks.py'] + self.action = 'list' + self.environs = [] + self.more_options = ['-n', 'NoPrgEnvCheck'] + returncode, stdout, _ = self._run_reframe() + self.assertIn('Found 0 check(s)', stdout) + self.assertEqual(0, returncode) From 771c641a92f61dd51283ead1f05dc2bc34c60a22 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Tue, 12 Jun 2018 09:49:51 +0200 Subject: [PATCH 2/6] Address PR comments * Create `allx` function in `reframe/utility/__init__.py`. * Create the corresponding deferrable version of `allx`. * Create the unit tests for the above functions. * Use raw strings in some regular expressions to avoid warnings. --- reframe/frontend/cli.py | 6 +- reframe/utility/__init__.py | 24 +++++++ reframe/utility/sanity.py | 11 +++ unittests/test_sanity_functions.py | 111 +++++++++++++++++------------ unittests/test_utility.py | 12 ++++ 5 files changed, 114 insertions(+), 50 deletions(-) diff --git a/reframe/frontend/cli.py b/reframe/frontend/cli.py index 47007cbf78..d738490e63 100644 --- a/reframe/frontend/cli.py +++ b/reframe/frontend/cli.py @@ -6,6 +6,7 @@ import reframe.core.config as config import reframe.core.logging as logging import reframe.core.runtime as runtime +import reframe.utility as util import reframe.utility.os_ext as os_ext from reframe.core.exceptions import (EnvironError, ConfigError, ReframeError, ReframeFatalError, format_exception, @@ -351,9 +352,8 @@ def main(): # Filter checks by prgenv if not options.skip_prgenv_check: checks_matched = filter( - lambda c: c if (c.valid_prog_environs - and all(c.supports_environ(e) - for e in options.prgenv)) else None, + lambda c: c if util.allx(c.supports_environ(e) + for e in options.prgenv) else None, checks_matched ) diff --git a/reframe/utility/__init__.py b/reframe/utility/__init__.py index d1f603ad63..a7e9b39cdd 100644 --- a/reframe/utility/__init__.py +++ b/reframe/utility/__init__.py @@ -1,9 +1,11 @@ import collections import importlib import importlib.util +import itertools import os import re import sys +import types from collections import UserDict @@ -54,6 +56,28 @@ def import_module_from_file(filename): return importlib.import_module(module_name) +def allx(iterable): + """Return ``True`` if all elements of the `iterable` are true. If + iterable is empty or ``None`` return ``False``. + + .. versionadded:: 2.13 + + """ + + # Check if iterable is an empty generator + if isinstance(iterable, types.GeneratorType): + try: + first_item = next(iterable) + return all(itertools.chain([first_item], iterable)) + except StopIteration: + return False + + if not iterable: + return False + + return all(iterable) + + def decamelize(s): """Decamelize the string ``s``. diff --git a/reframe/utility/sanity.py b/reframe/utility/sanity.py index 544288e8ee..d51d9785cb 100644 --- a/reframe/utility/sanity.py +++ b/reframe/utility/sanity.py @@ -63,6 +63,7 @@ import itertools import re +import reframe.utility as util from reframe.core.deferrable import deferrable, evaluate from reframe.core.exceptions import SanityError @@ -116,6 +117,16 @@ def all(iterable): return builtins.all(iterable) +@deferrable +def allx(iterable): + """Replacement for the :func:`allx() ` function. + + .. versionadded:: 2.13 + + """ + return util.allx(iterable) + + @deferrable def any(iterable): """Replacement for the built-in :func:`any() ` function.""" diff --git a/unittests/test_sanity_functions.py b/unittests/test_sanity_functions.py index 5591d91e8f..4e6f020d53 100644 --- a/unittests/test_sanity_functions.py +++ b/unittests/test_sanity_functions.py @@ -55,6 +55,23 @@ def test_all(self): l[2] = 1 self.assertTrue(expr) + def test_allx(self): + l1 = [1, 1, 1] + l2 = [1, 0] + expr1 = sn.allx(l1) + expr2 = sn.allx(l2) + expr3 = sn.allx([]) + expr4 = sn.allx(None) + expr5 = sn.allx(i for i in []) + expr6 = sn.allx(i for i in [1]) + self.assertTrue(expr1) + self.assertFalse(expr2) + self.assertFalse(expr3) + self.assertFalse(expr4) + self.assertFalse(expr5) + self.assertTrue(expr6) + return + def test_any(self): l = [0, 0, 1] expr = sn.any(l) @@ -167,9 +184,9 @@ def test_assert_true(self): evaluate, sn.assert_true(False)) self.assertRaisesRegex(SanityError, '0 is not True', evaluate, sn.assert_true(0)) - self.assertRaisesRegex(SanityError, '\[\] is not True', + self.assertRaisesRegex(SanityError, r'\[\] is not True', evaluate, sn.assert_true([])) - self.assertRaisesRegex(SanityError, 'range\(.+\) is not True', + self.assertRaisesRegex(SanityError, r'range\(.+\) is not True', evaluate, sn.assert_true(range(0))) self.assertRaisesRegex(SanityError, 'not true', evaluate, sn.assert_true(0, msg='not true')) @@ -182,7 +199,7 @@ def test_assert_true_with_deferrables(self): evaluate, sn.assert_true(make_deferrable(False))) self.assertRaisesRegex(SanityError, '0 is not True', evaluate, sn.assert_true(make_deferrable(0))) - self.assertRaisesRegex(SanityError, '\[\] is not True', + self.assertRaisesRegex(SanityError, r'\[\] is not True', evaluate, sn.assert_true(make_deferrable([]))) def test_assert_false(self): @@ -194,9 +211,9 @@ def test_assert_false(self): evaluate, sn.assert_false(True)) self.assertRaisesRegex(SanityError, '1 is not False', evaluate, sn.assert_false(1)) - self.assertRaisesRegex(SanityError, '\[1\] is not False', + self.assertRaisesRegex(SanityError, r'\[1\] is not False', evaluate, sn.assert_false([1])) - self.assertRaisesRegex(SanityError, 'range\(.+\) is not False', + self.assertRaisesRegex(SanityError, r'range\(.+\) is not False', evaluate, sn.assert_false(range(1))) def test_assert_false_with_deferrables(self): @@ -207,7 +224,7 @@ def test_assert_false_with_deferrables(self): evaluate, sn.assert_false(make_deferrable(True))) self.assertRaisesRegex(SanityError, '1 is not False', evaluate, sn.assert_false(make_deferrable(1))) - self.assertRaisesRegex(SanityError, '\[1\] is not False', + self.assertRaisesRegex(SanityError, r'\[1\] is not False', evaluate, sn.assert_false(make_deferrable([1]))) def test_assert_eq(self): @@ -291,24 +308,24 @@ def test_assert_le_with_deferrables(self): def test_assert_in(self): self.assertTrue(sn.assert_in(1, [1, 2, 3])) - self.assertRaisesRegex(SanityError, '0 is not in \[1, 2, 3\]', + self.assertRaisesRegex(SanityError, r'0 is not in \[1, 2, 3\]', evaluate, sn.assert_in(0, [1, 2, 3])) def test_assert_in_with_deferrables(self): self.assertTrue(sn.assert_in(1, make_deferrable([1, 2, 3]))) self.assertRaisesRegex( - SanityError, '0 is not in \[1, 2, 3\]', + SanityError, r'0 is not in \[1, 2, 3\]', evaluate, sn.assert_in(0, make_deferrable([1, 2, 3]))) def test_assert_not_in(self): self.assertTrue(sn.assert_not_in(0, [1, 2, 3])) - self.assertRaisesRegex(SanityError, '1 is in \[1, 2, 3\]', + self.assertRaisesRegex(SanityError, r'1 is in \[1, 2, 3\]', evaluate, sn.assert_not_in(1, [1, 2, 3])) def test_assert_not_in_with_deferrables(self): self.assertTrue(sn.assert_not_in(0, make_deferrable([1, 2, 3]))) self.assertRaisesRegex( - SanityError, '1 is in \[1, 2, 3\]', + SanityError, r'1 is in \[1, 2, 3\]', evaluate, sn.assert_not_in(1, make_deferrable([1, 2, 3]))) def test_assert_bounded(self): @@ -317,13 +334,13 @@ def test_assert_bounded(self): self.assertTrue(sn.assert_bounded(1, lower=-1.5)) self.assertTrue(sn.assert_bounded(1)) self.assertRaisesRegex(SanityError, - 'value 1 not within bounds -0\.5\.\.0\.5', + r'value 1 not within bounds -0\.5\.\.0\.5', evaluate, sn.assert_bounded(1, -0.5, 0.5)) self.assertRaisesRegex(SanityError, - 'value 1 not within bounds -inf\.\.0\.5', + r'value 1 not within bounds -inf\.\.0\.5', evaluate, sn.assert_bounded(1, upper=0.5)) self.assertRaisesRegex(SanityError, - 'value 1 not within bounds 1\.5\.\.inf', + r'value 1 not within bounds 1\.5\.\.inf', evaluate, sn.assert_bounded(1, lower=1.5)) self.assertRaisesRegex( SanityError, 'value 1 is out of bounds', evaluate, @@ -347,48 +364,48 @@ def test_assert_reference(self): self.assertRaisesRegex( SanityError, - '0\.5 is beyond reference value 1 \(l=0\.8, u=1\.1\)', + r'0\.5 is beyond reference value 1 \(l=0\.8, u=1\.1\)', evaluate, sn.assert_reference(0.5, 1, -0.2, 0.1) ) self.assertRaisesRegex( SanityError, - '0\.5 is beyond reference value 1 \(l=0\.8, u=inf\)', + r'0\.5 is beyond reference value 1 \(l=0\.8, u=inf\)', evaluate, sn.assert_reference(0.5, 1, -0.2) ) self.assertRaisesRegex( SanityError, - '1\.5 is beyond reference value 1 \(l=0\.8, u=1\.1\)', + r'1\.5 is beyond reference value 1 \(l=0\.8, u=1\.1\)', evaluate, sn.assert_reference(1.5, 1, -0.2, 0.1) ) self.assertRaisesRegex( SanityError, - '1\.5 is beyond reference value 1 \(l=-inf, u=1\.1\)', + r'1\.5 is beyond reference value 1 \(l=-inf, u=1\.1\)', evaluate, sn.assert_reference( 1.5, 1, lower_thres=None, upper_thres=0.1) ) self.assertRaisesRegex( SanityError, - '-0\.8 is beyond reference value -1 \(l=-1\.2, u=-0\.9\)', + r'-0\.8 is beyond reference value -1 \(l=-1\.2, u=-0\.9\)', evaluate, sn.assert_reference(-0.8, -1, -0.2, 0.1) ) # Check invalid thresholds self.assertRaisesRegex(SanityError, - 'invalid high threshold value: -0\.1', + r'invalid high threshold value: -0\.1', evaluate, sn.assert_reference(0.9, 1, -0.2, -0.1)) self.assertRaisesRegex(SanityError, - 'invalid low threshold value: 0\.2', + r'invalid low threshold value: 0\.2', evaluate, sn.assert_reference(0.9, 1, 0.2, 0.1)) self.assertRaisesRegex(SanityError, - 'invalid low threshold value: 1\.2', + r'invalid low threshold value: 1\.2', evaluate, sn.assert_reference(0.9, 1, 1.2, 0.1)) # check invalid thresholds greater than 1 self.assertRaisesRegex(SanityError, - 'invalid low threshold value: -2\.0', + r'invalid low threshold value: -2\.0', evaluate, sn.assert_reference(0.9, 1, -2.0, 0.1)) self.assertRaisesRegex(SanityError, - 'invalid high threshold value: 1\.5', + r'invalid high threshold value: 1\.5', evaluate, sn.assert_reference(-1.5, -1, -0.5, 1.5)) def _write_tempfile(self): @@ -403,11 +420,11 @@ def _write_tempfile(self): def test_assert_found(self): tempfile = self._write_tempfile() - self.assertTrue(sn.assert_found('Step: \d+', tempfile)) + self.assertTrue(sn.assert_found(r'Step: \d+', tempfile)) self.assertTrue(sn.assert_found( - 'Step: \d+', make_deferrable(tempfile))) + r'Step: \d+', make_deferrable(tempfile))) self.assertRaises(SanityError, evaluate, - sn.assert_found('foo: \d+', tempfile)) + sn.assert_found(r'foo: \d+', tempfile)) os.remove(tempfile) def test_assert_found_encoding(self): @@ -417,17 +434,17 @@ def test_assert_found_encoding(self): def test_assert_not_found(self): tempfile = self._write_tempfile() - self.assertTrue(sn.assert_not_found('foo: \d+', tempfile)) + self.assertTrue(sn.assert_not_found(r'foo: \d+', tempfile)) self.assertTrue( - sn.assert_not_found('foo: \d+', make_deferrable(tempfile)) + sn.assert_not_found(r'foo: \d+', make_deferrable(tempfile)) ) self.assertRaises(SanityError, evaluate, - sn.assert_not_found('Step: \d+', tempfile)) + sn.assert_not_found(r'Step: \d+', tempfile)) os.remove(tempfile) def test_assert_not_found_encoding(self): self.assertTrue( - sn.assert_not_found('Iliad', self.utf16_file, encoding='utf-16') + sn.assert_not_found(r'Iliad', self.utf16_file, encoding='utf-16') ) @@ -507,7 +524,7 @@ def tearDown(self): os.remove(self.tempfile) def test_findall(self): - res = evaluate(sn.findall('Step: \d+', self.tempfile)) + res = evaluate(sn.findall(r'Step: \d+', self.tempfile)) self.assertEqual(3, len(res)) res = evaluate(sn.findall('Step:.*', self.tempfile)) @@ -521,60 +538,60 @@ def test_findall(self): self.assertEqual(expected, match.group(0)) # Check groups - res = evaluate(sn.findall('Step: (?P\d+)', self.tempfile)) + res = evaluate(sn.findall(r'Step: (?P\d+)', self.tempfile)) for step, match in enumerate(res, start=1): self.assertEqual(step, int(match.group(1))) self.assertEqual(step, int(match.group('no'))) def test_findall_encoding(self): res = evaluate( - sn.findall(r'Odyssey', self.utf16_file, encoding='utf-16') + sn.findall('Odyssey', self.utf16_file, encoding='utf-16') ) self.assertEqual(1, len(res)) def test_findall_error(self): self.assertRaises(SanityError, evaluate, - sn.findall('Step: \d+', 'foo.txt')) + sn.findall(r'Step: \d+', 'foo.txt')) def test_extractall(self): # Check numeric groups - res = evaluate(sn.extractall('Step: (?P\d+)', self.tempfile, 1)) + res = evaluate(sn.extractall(r'Step: (?P\d+)', self.tempfile, 1)) for expected, v in enumerate(res, start=1): self.assertEqual(str(expected), v) # Check named groups - res = evaluate(sn.extractall('Step: (?P\d+)', self.tempfile, 'no')) + res = evaluate(sn.extractall(r'Step: (?P\d+)', self.tempfile, 'no')) for expected, v in enumerate(res, start=1): self.assertEqual(str(expected), v) # Check convert function - res = evaluate(sn.extractall('Step: (?P\d+)', + res = evaluate(sn.extractall(r'Step: (?P\d+)', self.tempfile, 'no', int)) for expected, v in enumerate(res, start=1): self.assertEqual(expected, v) def test_extractall_encoding(self): res = evaluate( - sn.extractall(r'Odyssey', self.utf16_file, encoding='utf-16') + sn.extractall('Odyssey', self.utf16_file, encoding='utf-16') ) self.assertEqual(1, len(res)) def test_extractall_error(self): self.assertRaises(SanityError, evaluate, - sn.extractall('Step: (\d+)', 'foo.txt', 1)) + sn.extractall(r'Step: (\d+)', 'foo.txt', 1)) self.assertRaises( SanityError, evaluate, - sn.extractall('Step: (?P\d+)', + sn.extractall(r'Step: (?P\d+)', self.tempfile, conv=int) ) self.assertRaises(SanityError, evaluate, - sn.extractall('Step: (\d+)', self.tempfile, 2)) + sn.extractall(r'Step: (\d+)', self.tempfile, 2)) self.assertRaises( SanityError, evaluate, - sn.extractall('Step: (?P\d+)', self.tempfile, 'foo')) + sn.extractall(r'Step: (?P\d+)', self.tempfile, 'foo')) def test_extractall_custom_conv(self): - res = evaluate(sn.extractall('Step: (\d+)', self.tempfile, 1, + res = evaluate(sn.extractall(r'Step: (\d+)', self.tempfile, 1, lambda x: int(x))) for expected, v in enumerate(res, start=1): self.assertEqual(expected, v) @@ -582,7 +599,7 @@ def test_extractall_custom_conv(self): # Check error in custom function self.assertRaises( SanityError, evaluate, - sn.extractall('Step: (\d+)', self.tempfile, + sn.extractall(r'Step: (\d+)', self.tempfile, conv=lambda x: int(x)) ) @@ -593,20 +610,20 @@ def __call__(self, x): self.assertRaises( SanityError, evaluate, - sn.extractall('Step: (\d+)', self.tempfile, conv=C()) + sn.extractall(r'Step: (\d+)', self.tempfile, conv=C()) ) def test_extractsingle(self): for i in range(1, 4): self.assertEqual( i, - sn.extractsingle('Step: (\d+)', self.tempfile, 1, int, i-1) + sn.extractsingle(r'Step: (\d+)', self.tempfile, 1, int, i-1) ) # Test out of bounds access self.assertRaises( SanityError, evaluate, - sn.extractsingle('Step: (\d+)', self.tempfile, 1, int, 100) + sn.extractsingle(r'Step: (\d+)', self.tempfile, 1, int, 100) ) def test_extractsingle_encoding(self): diff --git a/unittests/test_utility.py b/unittests/test_utility.py index 4cb2576c57..8799a9a17c 100644 --- a/unittests/test_utility.py +++ b/unittests/test_utility.py @@ -365,6 +365,18 @@ def tearDown(self): class TestMiscUtilities(unittest.TestCase): + def test_allx(self): + l1 = [1, 1, 1] + l2 = [True, False] + self.assertTrue(all(l1), util.allx(l1)) + self.assertFalse(all(l2), util.allx(l2)) + self.assertFalse(util.allx([])) + self.assertFalse(util.allx(None)) + self.assertTrue(util.allx(i for i in [1, 1, 1])) + self.assertTrue(util.allx(i for i in [1])) + self.assertFalse(util.allx(i for i in [0])) + self.assertFalse(util.allx(i for i in [])) + def test_decamelize(self): self.assertEqual('', util.decamelize('')) self.assertEqual('my_base_class', util.decamelize('MyBaseClass')) From 142f89a026d00b50e9698de367680ded12514052 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Tue, 12 Jun 2018 13:55:31 +0200 Subject: [PATCH 3/6] Addres PR comments (version 2) --- reframe/utility/__init__.py | 19 +++++++++++-------- reframe/utility/sanity.py | 2 +- unittests/test_sanity_functions.py | 22 +++++++--------------- unittests/test_utility.py | 9 +++++---- 4 files changed, 24 insertions(+), 28 deletions(-) diff --git a/reframe/utility/__init__.py b/reframe/utility/__init__.py index a7e9b39cdd..dbd5c645a8 100644 --- a/reframe/utility/__init__.py +++ b/reframe/utility/__init__.py @@ -57,25 +57,28 @@ def import_module_from_file(filename): def allx(iterable): - """Return ``True`` if all elements of the `iterable` are true. If - iterable is empty or ``None`` return ``False``. + """Same as the built-in all, except that it returns :class:`False` if + ``iterable`` is empty. .. versionadded:: 2.13 """ - # Check if iterable is an empty generator + # The following code first tests that `iterable` is a generator and + # then checks if the generator has at least one element into it. if isinstance(iterable, types.GeneratorType): try: - first_item = next(iterable) - return all(itertools.chain([first_item], iterable)) + head = next(iterable) except StopIteration: return False + else: + return all(itertools.chain([head], iterable)) - if not iterable: - return False + if not isinstance(iterable, collections.abc.Iterable): + raise TypeError("'%s' object is not iterable" + % iterable.__class__.__name__) - return all(iterable) + return all(iterable) if iterable else False def decamelize(s): diff --git a/reframe/utility/sanity.py b/reframe/utility/sanity.py index d51d9785cb..d987180f3a 100644 --- a/reframe/utility/sanity.py +++ b/reframe/utility/sanity.py @@ -119,7 +119,7 @@ def all(iterable): @deferrable def allx(iterable): - """Replacement for the :func:`allx() ` function. + """Replacement for the :func:`allx()` function. .. versionadded:: 2.13 diff --git a/unittests/test_sanity_functions.py b/unittests/test_sanity_functions.py index 4e6f020d53..222519628a 100644 --- a/unittests/test_sanity_functions.py +++ b/unittests/test_sanity_functions.py @@ -56,21 +56,13 @@ def test_all(self): self.assertTrue(expr) def test_allx(self): - l1 = [1, 1, 1] - l2 = [1, 0] - expr1 = sn.allx(l1) - expr2 = sn.allx(l2) - expr3 = sn.allx([]) - expr4 = sn.allx(None) - expr5 = sn.allx(i for i in []) - expr6 = sn.allx(i for i in [1]) - self.assertTrue(expr1) - self.assertFalse(expr2) - self.assertFalse(expr3) - self.assertFalse(expr4) - self.assertFalse(expr5) - self.assertTrue(expr6) - return + self.assertTrue(sn.allx([1, 1, 1])) + self.assertFalse(sn.allx([1, 0])) + self.assertFalse(sn.allx([])) + self.assertFalse(sn.allx(i for i in range(0))) + self.assertTrue(sn.allx(i for i in range(1, 2))) + with self.assertRaises(TypeError): + sn.evaluate(sn.allx(None)) def test_any(self): l = [0, 0, 1] diff --git a/unittests/test_utility.py b/unittests/test_utility.py index 8799a9a17c..1f1066c24d 100644 --- a/unittests/test_utility.py +++ b/unittests/test_utility.py @@ -371,11 +371,12 @@ def test_allx(self): self.assertTrue(all(l1), util.allx(l1)) self.assertFalse(all(l2), util.allx(l2)) self.assertFalse(util.allx([])) - self.assertFalse(util.allx(None)) self.assertTrue(util.allx(i for i in [1, 1, 1])) - self.assertTrue(util.allx(i for i in [1])) - self.assertFalse(util.allx(i for i in [0])) - self.assertFalse(util.allx(i for i in [])) + self.assertTrue(util.allx(i for i in range(1, 2))) + self.assertFalse(util.allx(i for i in range(1))) + self.assertFalse(util.allx(i for i in range(0))) + with self.assertRaises(TypeError): + util.allx(None) def test_decamelize(self): self.assertEqual('', util.decamelize('')) From 4f4377cb4b9d3cbd375af663002d013d940c7f1a Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Tue, 12 Jun 2018 16:05:31 +0200 Subject: [PATCH 4/6] Code organisation fixes - Rationale on special treatment of generators in allx. - Document the sanity function correctly. - Correct line splits. --- reframe/utility/__init__.py | 11 ++++------- reframe/utility/sanity.py | 20 ++++++++++---------- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/reframe/utility/__init__.py b/reframe/utility/__init__.py index dbd5c645a8..19e96c9eb4 100644 --- a/reframe/utility/__init__.py +++ b/reframe/utility/__init__.py @@ -59,13 +59,10 @@ def import_module_from_file(filename): def allx(iterable): """Same as the built-in all, except that it returns :class:`False` if ``iterable`` is empty. - - .. versionadded:: 2.13 - """ - # The following code first tests that `iterable` is a generator and - # then checks if the generator has at least one element into it. + # Generators must be treated specially, because there is no way to get + # their size without consuming their elements. if isinstance(iterable, types.GeneratorType): try: head = next(iterable) @@ -75,8 +72,8 @@ def allx(iterable): return all(itertools.chain([head], iterable)) if not isinstance(iterable, collections.abc.Iterable): - raise TypeError("'%s' object is not iterable" - % iterable.__class__.__name__) + raise TypeError("'%s' object is not iterable" % + iterable.__class__.__name__) return all(iterable) if iterable else False diff --git a/reframe/utility/sanity.py b/reframe/utility/sanity.py index d987180f3a..18472d3415 100644 --- a/reframe/utility/sanity.py +++ b/reframe/utility/sanity.py @@ -117,16 +117,6 @@ def all(iterable): return builtins.all(iterable) -@deferrable -def allx(iterable): - """Replacement for the :func:`allx()` function. - - .. versionadded:: 2.13 - - """ - return util.allx(iterable) - - @deferrable def any(iterable): """Replacement for the built-in :func:`any() ` function.""" @@ -673,6 +663,16 @@ def avg(iterable): # Other utility functions +@deferrable +def allx(iterable): + """Same as the built-in :func:`all() ` function, except that it + returns :class:`False` if ``iterable`` is empty. + + .. versionadded:: 2.13 + """ + return util.allx(iterable) + + @deferrable def getitem(container, item): """Get ``item`` from ``container``. From e1e5a43b83bec9218fe42381b0a9700d385e38c0 Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Thu, 14 Jun 2018 15:07:45 +0200 Subject: [PATCH 5/6] Check for empty 'options.prgenv' --- reframe/frontend/cli.py | 15 ++++++++++----- unittests/test_cli.py | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/reframe/frontend/cli.py b/reframe/frontend/cli.py index d738490e63..53a4bb6cc3 100644 --- a/reframe/frontend/cli.py +++ b/reframe/frontend/cli.py @@ -351,11 +351,16 @@ def main(): # Filter checks by prgenv if not options.skip_prgenv_check: - checks_matched = filter( - lambda c: c if util.allx(c.supports_environ(e) - for e in options.prgenv) else None, - checks_matched - ) + if options.prgenv: + filter_func = lambda c: c if util.allx( + c.supports_environ(e) for e in options.prgenv) else None + else: + # Here we also check that the valid_prog_environs of the check + # is an iterable and not empty + filter_func = lambda c: c if util.allx( + c.valid_prog_environs) else None + + checks_matched = filter(filter_func, checks_matched) # Filter checks further if options.gpu_only and options.cpu_only: diff --git a/unittests/test_cli.py b/unittests/test_cli.py index b355e7d000..d40206ca7c 100644 --- a/unittests/test_cli.py +++ b/unittests/test_cli.py @@ -318,7 +318,25 @@ def test_timestamp_option(self): self.assertNotEqual(0, returncode) self.assertIn(timefmt, stdout) + def test_list_empty_prgenvs_check_and_options(self): + self.checkpath = ['unittests/resources/checks/frontend_checks.py'] + self.action = 'list' + self.environs = [] + self.more_options = ['-n', 'NoPrgEnvCheck'] + returncode, stdout, _ = self._run_reframe() + self.assertIn('Found 0 check(s)', stdout) + self.assertEqual(0, returncode) + def test_list_check_with_empty_prgenvs(self): + self.checkpath = ['unittests/resources/checks/frontend_checks.py'] + self.action = 'list' + self.environs = ['foo'] + self.more_options = ['-n', 'NoPrgEnvCheck'] + returncode, stdout, _ = self._run_reframe() + self.assertIn('Found 0 check(s)', stdout) + self.assertEqual(0, returncode) + + def test_list_empty_prgenvs_in_check_and_options(self): self.checkpath = ['unittests/resources/checks/frontend_checks.py'] self.action = 'list' self.environs = [] From eb34e054623f7f30ea7c32c3563993252d58dbda Mon Sep 17 00:00:00 2001 From: Theofilos Manitaras Date: Thu, 14 Jun 2018 20:15:45 +0200 Subject: [PATCH 6/6] Address PR comments (version 3) --- reframe/frontend/cli.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/reframe/frontend/cli.py b/reframe/frontend/cli.py index 53a4bb6cc3..c6224f7836 100644 --- a/reframe/frontend/cli.py +++ b/reframe/frontend/cli.py @@ -350,17 +350,14 @@ def main(): ) # Filter checks by prgenv - if not options.skip_prgenv_check: + def filter_prgenv(c): if options.prgenv: - filter_func = lambda c: c if util.allx( - c.supports_environ(e) for e in options.prgenv) else None + return util.allx(c.supports_environ(e) for e in options.prgenv) else: - # Here we also check that the valid_prog_environs of the check - # is an iterable and not empty - filter_func = lambda c: c if util.allx( - c.valid_prog_environs) else None + return bool(c.valid_prog_environs) - checks_matched = filter(filter_func, checks_matched) + if not options.skip_prgenv_check: + checks_matched = filter(filter_prgenv, checks_matched) # Filter checks further if options.gpu_only and options.cpu_only: