Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filter out tests with no programming environments #319

Merged
merged 9 commits into from
Jun 15, 2018
16 changes: 11 additions & 5 deletions reframe/frontend/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -350,11 +351,16 @@ 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,
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest replacing the lambdas altogether here in favour of a closure:

def filter_prgenv(c):
    if options.prgenv:
        return util.allx(c.supports_environ(e) for e in options.prgenv)
    else:
        return bool(c.valid_prog_environs)


checks_matched = filter(filter_func, checks_matched)

# Filter checks further
if options.gpu_only and options.cpu_only:
Expand Down
24 changes: 24 additions & 0 deletions reframe/utility/__init__.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -58,6 +60,28 @@ def import_module_from_file(filename):
return importlib.import_module(module_name)


def allx(iterable):
"""Same as the built-in all, except that it returns :class:`False` if
``iterable`` is empty.
"""

# 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)
except StopIteration:
return False
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't understand all this fuss here. Why a check of iterable against collections.abc.Iterable wouldn't just work?

>>> import collections
>>> isinstance(range(0), collections.abc.Iterable)
True
>>>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we do not want to accept empty generators. Is there any other way to check that a generator is empty?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by "empty generator"? A generator is a generator, i.e., an iterable, even if it doesn't produce any values. Your unit tests should pass fine using my proposal.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean a generator like (i for i in []) if it is passed to allx how can I check that there are no elements and return False?

Copy link
Contributor

@vkarak vkarak Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now, I get what you are trying to do. The comment above the code is really misleading... Please change it with sth more precise!

else:
return all(itertools.chain([head], iterable))

if not isinstance(iterable, collections.abc.Iterable):
raise TypeError("'%s' object is not iterable" %
iterable.__class__.__name__)

return all(iterable) if iterable else False


def decamelize(s):
"""Decamelize the string ``s``.

Expand Down
11 changes: 11 additions & 0 deletions reframe/utility/sanity.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -662,6 +663,16 @@ def avg(iterable):

# Other utility functions

@deferrable
def allx(iterable):
"""Same as the built-in :func:`all() <python: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``.
Expand Down
33 changes: 30 additions & 3 deletions unittests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -317,3 +317,30 @@ def test_timestamp_option(self):
returncode, stdout, _ = self._run_reframe()
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 = []
self.more_options = ['-n', 'NoPrgEnvCheck']
returncode, stdout, _ = self._run_reframe()
self.assertIn('Found 0 check(s)', stdout)
self.assertEqual(0, returncode)
Loading