Skip to content

Commit 1cf41f6

Browse files
ZELTROX1Copilot
andauthored
Handle missing reference results gracefully in Systemtest field comparison (#717)
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent a6243c4 commit 1cf41f6

2 files changed

Lines changed: 47 additions & 13 deletions

File tree

changelog-entries/404.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- Improved system tests to fail cleanly with a clear error message when reference result archives are missing or invalid, instead of crashing with a file-related exception.

tools/tests/systemtests/Systemtest.py

Lines changed: 46 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import subprocess
2-
from typing import List, Dict, Optional
2+
from typing import List, Dict, Optional, Tuple
33
from jinja2 import Environment, FileSystemLoader
44
from dataclasses import dataclass, field
55
import shutil
@@ -353,26 +353,59 @@ def __write_env_file(self):
353353
for key, value in self.env.items():
354354
env_file.write(f"{key}={value}\n")
355355

356-
def __unpack_reference_results(self):
357-
with tarfile.open(self.reference_result.path) as reference_results_tared:
358-
# specify which folder to extract to
359-
reference_results_tared.extractall(self.system_test_dir / PRECICE_REL_REFERENCE_DIR)
360-
logging.debug(
361-
f"extracting {self.reference_result.path} into {self.system_test_dir / PRECICE_REL_REFERENCE_DIR}")
356+
def __unpack_reference_results(self) -> Tuple[bool, str]:
357+
if not self.reference_result.path.exists():
358+
error_message = (
359+
f"Reference results archive was not found for {self}. "
360+
f"Expected file: {self.reference_result.path}. "
361+
"Please generate the reference results first or update tests.yaml accordingly.")
362+
logging.error(error_message)
363+
return False, error_message
364+
365+
try:
366+
# Base directory where reference results should be extracted
367+
dest_dir = self.system_test_dir / PRECICE_REL_REFERENCE_DIR
368+
dest_dir.mkdir(parents=True, exist_ok=True)
369+
dest_dir_resolved = dest_dir.resolve()
370+
371+
with tarfile.open(self.reference_result.path) as reference_results_tared:
372+
# Validate that each member will be extracted within dest_dir
373+
for member in reference_results_tared.getmembers():
374+
member_path = dest_dir / member.name
375+
member_path_resolved = member_path.resolve()
376+
# Ensure the resolved member path is within the destination directory
377+
if os.path.commonpath([str(dest_dir_resolved), str(
378+
member_path_resolved)]) != str(dest_dir_resolved):
379+
logging.error(
380+
f"Unsafe path detected in reference results archive {self.reference_result.path} "
381+
f"for {self}: {member.name}")
382+
return False
383+
384+
# All paths are safe; extract into the destination directory
385+
reference_results_tared.extractall(dest_dir)
386+
387+
logging.debug(
388+
f"extracting {self.reference_result.path} into {dest_dir}")
389+
return True, ""
390+
except (tarfile.TarError, OSError) as e:
391+
error_message = (
392+
f"Could not unpack reference results archive {self.reference_result.path} for {self}: {e}")
393+
logging.error(error_message)
394+
return False, error_message
362395

363396
def _run_field_compare(self):
364397
"""
365-
Writes the Docker Compose file to disk, executes docker-compose up, and handles the process output.
366-
367-
Args:
368-
docker_compose_content: The content of the Docker Compose file.
398+
Executes the field comparison step after unpacking reference results.
369399
370400
Returns:
371-
A SystemtestResult object containing the state.
401+
A FieldCompareResult object containing the command outcome and logs.
372402
"""
373403
logging.debug(f"Running fieldcompare for {self}")
374404
time_start = time.perf_counter()
375-
self.__unpack_reference_results()
405+
unpack_success, unpack_error_message = self.__unpack_reference_results()
406+
if not unpack_success:
407+
elapsed_time = time.perf_counter() - time_start
408+
return FieldCompareResult(1, [], [unpack_error_message], self, elapsed_time)
376409
docker_compose_content = self.__get_field_compare_compose_file()
377410
stdout_data = []
378411
stderr_data = []

0 commit comments

Comments
 (0)