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

Make test shell wrapper filename even more unique #2998

Merged
merged 1 commit into from
Jun 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
23 changes: 19 additions & 4 deletions tmt/steps/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
field,
key_to_option,
option_to_key,
render_template,
)

if TYPE_CHECKING:
Expand Down Expand Up @@ -1227,7 +1228,7 @@ def safe_name(self) -> str:
slash characters, ``/``.
"""

return tmt.utils.sanitize_name(self.name, allow_slash=False)
return self.pathless_safe_name

@classmethod
def base_command(
Expand Down Expand Up @@ -2290,11 +2291,11 @@ def sync_with_guests(
from failed_actions[0].exc


def safe_filename(basename: str, phase: Phase, guest: 'Guest') -> Path:
def safe_filename(template: str, phase: Phase, guest: 'Guest', **variables: Any) -> Path:
"""
Construct a non-conflicting filename safe for parallel tasks.

Function adds enough uniqueness to the starting base name by adding a phase
Function adds enough uniqueness to a given template by adding a phase
name and a guest name that the eventual filename would be safe against
conflicting access from a phase running on multiple guests, and against
reuse when created by the same plugin in different phases.
Expand All @@ -2318,6 +2319,20 @@ def safe_filename(basename: str, phase: Phase, guest: 'Guest') -> Path:
re-used by different plugin executions. Adding the phase name should
lower confusion: it would be immediately clear which phase used which
filename, or whether a filename was or was not created by given phase.

:param template: filename template. It is enhanced with ``phase``
and ``guest`` safe name, but may use other variables provided
in ``variables``.
:param phase: a phase owning the resulting filename.
:param guest: a guest on which the filename would be used.
:param variables: additional variables ``template`` need when
rendering the filename.
"""

return Path(f'{basename}-{phase.safe_name}-{guest.safe_name}')
template += '-{{ PHASE.safe_name }}-{{ GUEST.safe_name }}'

return Path(render_template(
template,
PHASE=phase,
GUEST=guest,
**variables))
14 changes: 12 additions & 2 deletions tmt/steps/execute/internal.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,16 @@ def effective_pidfile_root() -> Path:
return TEST_PIDFILE_ROOT


TEST_WRAPPER_FILENAME = 'tmt-test-wrapper.sh'
#: A template for the shell wrapper in which the test script is
#: saved.
#:
#: It is passed to :py:func:`tmt.utils.safe_filename`, but
#: includes also test name and serial number to make it unique
#: even among all test wrappers. See https:/teemtee/tmt/issues/2997
#: for issue motivating the inclusion, it seems to be a good idea
#: to prevent accidental reuse in general.
TEST_WRAPPER_FILENAME_TEMPLATE = \
'tmt-test-wrapper.sh-{{ INVOCATION.test.pathless_safe_name }}-{{ INVOCATION.test.serial_number }}' # noqa: E501

# tmt test wrapper is getting complex. Besides honoring the timeout
# and interactivity request, it also must play nicely with reboots
Expand Down Expand Up @@ -307,7 +316,8 @@ def execute(
# the same `discover` phase for different guests at the same time, and
# must keep them isolated. The wrapper script, while being prepared, is
# a shared global state, and we must prevent race conditions.
test_wrapper_filename = safe_filename(TEST_WRAPPER_FILENAME, self, guest)
test_wrapper_filename = safe_filename(
TEST_WRAPPER_FILENAME_TEMPLATE, self, guest, INVOCATION=invocation)
test_wrapper_filepath = workdir / test_wrapper_filename

logger.debug('test wrapper', test_wrapper_filepath)
Expand Down
13 changes: 13 additions & 0 deletions tmt/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -1570,10 +1570,23 @@ def safe_name(self) -> str:

Spaces and other special characters are removed to prevent problems with
tools which do not expect them (e.g. in directory names).

Unlike :py:meth:`pathless_safe_name`, this property preserves
slashes, ``/``.
"""

return sanitize_name(self.name)

@cached_property
def pathless_safe_name(self) -> str:
"""
A safe variant of the name which does not contain any special characters.

Unlike :py:attr:`safe_name`, this property removes even slashes, ``/``.
"""

return sanitize_name(self.name, allow_slash=False)

def __str__(self) -> str:
""" Name is the default string representation """
return self.name
Expand Down
Loading