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
5 changes: 3 additions & 2 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 @@ -351,8 +352,8 @@ 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 util.allx(c.supports_environ(e)
for e in options.prgenv) else None,
checks_matched
)

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 @@ -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``.
Copy link
Contributor

Choose a reason for hiding this comment

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

  • I would phrase this different to stress the connection with the builtin all(): Same as the built-in all, except that it returns False if iterable is empty.
  • This function must be fully equivalent with the built-in all() if iterable is not empty, which means, we should raise TypeError if the argument is None. Pay attention to have fully compatible behaviour.
  • There are several documentation formatting issues here:
    • Double backquotes must be used for citing function arguments, here iterable.
    • None, True and False must be formatted as :class:`None` , :class:`True` etc.


.. 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))
Copy link
Contributor

Choose a reason for hiding this comment

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

Better put this in the else block of the try/except. Also rename first_item to first or head.

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!


if not iterable:
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not enough. If iterable is None, we should raise a TypeError as all() does.

return False

return all(iterable)


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 @@ -116,6 +117,16 @@ def all(iterable):
return builtins.all(iterable)


@deferrable
def allx(iterable):
"""Replacement for the :func:`allx() <reframe.utility.allx>` function.
Copy link
Contributor

Choose a reason for hiding this comment

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

The documentation of reframe.utility is not public, so you may not refer to it here.


.. versionadded:: 2.13

"""
return util.allx(iterable)


@deferrable
def any(iterable):
"""Replacement for the built-in :func:`any() <python:any>` function."""
Expand Down
15 changes: 12 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,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)
Loading