Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ jobs:
reframe -vvv --show-config

# perform a dry run of *all* tests, without any filtering
time reframe --dry-run 2>&1 | tee dry_run.out
time reframe --dry-run -S readonly_files_undefined_policy=error -S required_mem_per_node_undefined_policy=error 2>&1 | tee dry_run.out
pattern='PASSED .*0 failure'
grep "${pattern}" dry_run.out || (echo "Pattern '${pattern}' not found!" && exit 1)

Expand Down
33 changes: 22 additions & 11 deletions eessi/testsuite/eessi_mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

from eessi.testsuite import hooks
from eessi.testsuite.constants import DEVICE_TYPES, SCALES, COMPUTE_UNITS, TAGS
from eessi.testsuite.utils import log
from eessi.testsuite.utils import log, log_once
from eessi.testsuite import __version__ as testsuite_version


Expand Down Expand Up @@ -45,6 +45,8 @@ class EESSI_Mixin(RegressionMixin):
exact_memory = variable(bool, value=False)
user_executable_opts = variable(str, value='')
thread_binding = variable(str, value='false')
required_mem_per_node_undefined_policy = variable(str, value='warning')
readonly_files_undefined_policy = variable(str, value='warning')

# Set defaults for these class variables, can be overwritten by child class if desired
scale = parameter(SCALES.keys())
Expand Down Expand Up @@ -80,14 +82,7 @@ def __init_subclass__(cls, **kwargs):
cls.valid_systems = ['*']
if not cls.time_limit:
cls.time_limit = '1h'
if not cls.readonly_files:
msg = ' '.join([
"Built-in attribute `readonly_files` is empty. To avoid excessive copying, it's highly recommended",
"to add all files and/or dirs in `sourcesdir` that are needed but not modified during the test,",
"thus can be symlinked into the stage dirs. If you are sure there are no such files,",
"set `readonly_files = ['']`.",
])
raise ReframeFatalError(msg)

if cls._rfm_local_param_space.get('scale'):
getlogger().verbose(f"Scales supported by {cls.__qualname__}: {cls._rfm_local_param_space['scale'].values}")

Expand All @@ -106,6 +101,19 @@ def EESSI_mixin_validate_item_in_list(self, item, valid_items):
msg = f"The variable '{item}' has value {value}, but the only valid values are {valid_items}"
raise ReframeFatalError(msg)

@run_after('init')
def EESSI_check_readonly_files(self):
if not self.readonly_files:
msg = ' '.join([
"Built-in attribute `readonly_files` is empty. To avoid excessive copying, it's highly recommended",
"to add all files and/or dirs in `sourcesdir` that are needed but not modified during the test,",
"thus can be symlinked into the stage dirs. If you are sure there are no such files,",
"set `readonly_files = ['']`.",
])
if self.readonly_files_undefined_policy == 'error':
raise ReframeFatalError(msg)
log_once(self, msg, msg_id='1', level=self.readonly_files_undefined_policy)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Won't this conflict with other messages with msg_id='1', such as

log_once(self, warn_msg, msg_id='1', level='warning')

I mean, setting a unique message ID within a test class is doable. But if you set a message ID within EESSI_mixin, it's very hard to guarantee that the same message ID won't be reused in a child-class, no?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or is the test.__class__.__name__ here something unique to EESSI_mixin? And then does this print once in total, i.e. even if multiple child classes fail to specify readonly_files?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it's been a while, but if i remember well test.__class__.__name__ is equal to EESSI_mixin here, so no conflicts with child classes


@run_after('init')
def EESSI_mixin_validate_init(self):
"""Check that all variables that have to be set for subsequent hooks in the init phase have been set"""
Expand Down Expand Up @@ -200,7 +208,9 @@ def EESSI_mixin_validate_setup(self):
msg += " from EESSI_Mixin before (or in) the setup phase, but it wasn't. Note that this function"
msg += " can use self.num_tasks_per_node, as it will be called after that attribute"
msg += " has been set."
raise ReframeFatalError(msg)
if self.required_mem_per_node_undefined_policy == 'error':
raise ReframeFatalError(msg)
log_once(self, msg, msg_id='2', level=self.required_mem_per_node_undefined_policy)

# Check that the value for these variables is valid
# i.e. exists in their respective dict from eessi.testsuite.constants
Expand All @@ -227,7 +237,8 @@ def EESSI_set_launcher(self):
@run_after('setup')
def EESSI_mixin_request_mem(self):
"""Call hook to request the required amount of memory per node"""
hooks.req_memory_per_node(self, app_mem_req=self.required_mem_per_node())
if hasattr(self, 'required_mem_per_node'):
hooks.req_memory_per_node(self, app_mem_req=self.required_mem_per_node())

@run_after('setup')
def EESSI_mixin_log_runtime_info(self):
Expand Down
2 changes: 1 addition & 1 deletion eessi/testsuite/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -340,4 +340,4 @@ def log_once(test: rfm.RegressionTest, msg: str, msg_id: str, level: str = 'info

_unique_msg_ids.append(unique_id)
loglevel = getattr(getlogger(), level)
loglevel(msg)
loglevel(f'{test.__class__.__name__}: {msg}')