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

Add (custom) result file validation, schema-based #3153

Merged
merged 4 commits into from
Aug 27, 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
6 changes: 6 additions & 0 deletions docs/releases.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@
Releases
======================

tmt-1.36
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

tmt will now emit a warning when :ref:`custom test results</spec/tests/result>`
file does not follow the :ref:`result specification</spec/plans/results>`.


tmt-1.35
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Expand Down
2 changes: 2 additions & 0 deletions tests/execute/result/custom.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ rlJournalStart
testName="/test/custom-results"
rlPhaseStartTest "${testName}"
rlRun -s "${tmt_command} ${testName} 2>&1 >/dev/null" 1 "Test provides 'results.yaml' file by itself"
rlAssertGrep "warn: Result format violation: 3.name - 'without-leading-slash' does not match '^/.*'" $rlRun_LOG
rlAssertGrep "00:11:22 pass /test/custom-results/test/passing" $rlRun_LOG
rlAssertGrep "00:22:33 fail /test/custom-results/test/failing" $rlRun_LOG
rlAssertGrep "00:00:00 skip /test/custom-results/test/skipped" $rlRun_LOG
Expand Down Expand Up @@ -94,6 +95,7 @@ rlJournalStart
rlPhaseStartTest "${testName}"
rlRun -s "${tmt_command} ${testName} 2>&1 >/dev/null" 2 "Test provides partial result with wrong value"
rlAssertGrep "Invalid partial custom result 'errrrrr'." $rlRun_LOG
rlAssertGrep "warn: Result format violation: 0.result - 'errrrrr' is not one of \\['pass', 'fail', 'info', 'warn', 'error', 'skip'\\]" $rlRun_LOG
rlPhaseEnd

rlPhaseStartCleanup
Expand Down
2 changes: 2 additions & 0 deletions tmt/schemas/common.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,7 @@ definitions:
- info
- warn
- error
- skip

# https://tmt.readthedocs.io/en/stable/spec/tests.html#result
result_interpret:
Expand All @@ -477,6 +478,7 @@ definitions:
- warn
- error
- fail
- skip
- custom
- restraint

Expand Down
19 changes: 12 additions & 7 deletions tmt/schemas/results.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ definitions:
$ref: "/schemas/common#/definitions/timestamp"

duration:
$ref: "/definitions/duration"
$ref: "#/definitions/duration"

required:
- name
Expand All @@ -52,7 +52,7 @@ definitions:
check_results:
type: array
items:
$ref: "/definitions/check_result"
$ref: "#/definitions/check_result"

subresult:
type: object
Expand All @@ -76,15 +76,20 @@ definitions:
$ref: "/schemas/common#/definitions/timestamp"

duration:
$ref: "/definitions/duration"
$ref: "#/definitions/duration"

check:
$ref: "/definitions/check_results"
$ref: "#/definitions/check_results"

required:
- name
- result

subresults:
type: array
items:
$ref: "#/definitions/subresult"

type: array
items:
type: object
Expand Down Expand Up @@ -128,7 +133,7 @@ items:
$ref: "/schemas/common#/definitions/timestamp"

duration:
$ref: "/definitions/duration"
$ref: "#/definitions/duration"

ids:
type: object
Expand All @@ -140,10 +145,10 @@ items:
$ref: "/schemas/common#/definitions/result_log"

check:
$ref: "/definitions/check_results"
$ref: "#/definitions/check_results"

subresult:
$ref: "/definitions/subresult"
$ref: "#/definitions/subresults"

required:
- name
Expand Down
37 changes: 34 additions & 3 deletions tmt/steps/execute/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

import click
import fmf
import fmf.utils

import tmt
import tmt.base
Expand Down Expand Up @@ -446,10 +447,33 @@ def terminate_process(
class ResultCollection:
""" Collection of raw results loaded from a file """

invocation: TestInvocation

filepaths: list[Path]
file_exists: bool = False
results: list['tmt.result.RawResult'] = dataclasses.field(default_factory=list)

def validate(self) -> None:
"""
Validate raw collected results against the result JSON schema.

Report found errors as warnings via :py:attr:`invocation` logger.
"""

schema = tmt.utils.load_schema(Path('results.yaml'))
schema_store = tmt.utils.load_schema_store()

result = fmf.utils.validate_data(self.results, schema, schema_store=schema_store)

if not result.errors:
self.invocation.logger.debug('Results successfully validated.', level=4, shift=1)

return

for _, error in tmt.utils.preformat_jsonschema_validation_errors(result.errors):
self.invocation.logger.warning(
f'Result format violation: {error}', shift=1)


class ExecutePlugin(tmt.steps.Plugin[ExecuteStepDataT, None]):
""" Common parent of execute plugins """
Expand Down Expand Up @@ -594,8 +618,9 @@ def _load_custom_results_file(self, invocation: TestInvocation) -> ResultCollect
custom_results_path_yaml = invocation.test_data_path / 'results.yaml'
custom_results_path_json = invocation.test_data_path / 'results.json'

collection = ResultCollection(filepaths=[
custom_results_path_yaml, custom_results_path_json])
collection = ResultCollection(
invocation=invocation,
filepaths=[custom_results_path_yaml, custom_results_path_json])

if custom_results_path_yaml.exists():
with open(custom_results_path_yaml) as results_file:
Expand All @@ -621,7 +646,7 @@ def _load_tmt_report_results_file(self, invocation: TestInvocation) -> ResultCol
"""

results_path = self._tmt_report_results_filepath(invocation)
collection = ResultCollection(filepaths=[results_path])
collection = ResultCollection(invocation=invocation, filepaths=[results_path])

# Nothing to do if there's no result file
if not results_path.exists():
Expand Down Expand Up @@ -795,6 +820,8 @@ def extract_custom_results(self, invocation: TestInvocation) -> list["tmt.Result
note="no custom results were provided",
result=ResultOutcome.ERROR)]

collection.validate()

return self._process_results_partials(invocation, collection.results)

def extract_tmt_report_results(self, invocation: TestInvocation) -> list["tmt.Result"]:
Expand All @@ -819,6 +846,8 @@ def extract_tmt_report_results(self, invocation: TestInvocation) -> list["tmt.Re
raise tmt.utils.ExecuteError(
f"Test results not found in result file '{results_path}'.")

collection.validate()

return self._process_results_reduce(invocation, collection.results)

def extract_tmt_report_results_restraint(
Expand Down Expand Up @@ -847,6 +876,8 @@ def extract_tmt_report_results_restraint(
raise tmt.utils.ExecuteError(
f"Test results not found in result file '{results_path}'.")

collection.validate()

return self._process_results_partials(
invocation, collection.results, default_log=default_log)

Expand Down
45 changes: 32 additions & 13 deletions tmt/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5310,6 +5310,37 @@ def _process_step_collection(step_name: str, step_collection: Any) -> None:
return node


def preformat_jsonschema_validation_errors(
thrix marked this conversation as resolved.
Show resolved Hide resolved
raw_errors: list[jsonschema.ValidationError],
prefix: Optional[str] = None) -> list[tuple[jsonschema.ValidationError, str]]:
"""
A helper to preformat JSON schema validation errors.

Raw errors can be converted to strings with a simple ``str()`` call,
but resulting string is very JSON-ish. This helper provides
simplified string representation consisting of error message and
element path.

:param raw_error: raw validation errors as provided by
:py:mod:`jsonschema`.
:param prefix: if specified, it is added at the beginning of each
stringified error.
:returns: a list of two-item tuples, the first item being the
original validation error, the second item being its simplified
string rendering.
"""

prefix = f'{prefix}:' if prefix else ''
errors: list[tuple[jsonschema.ValidationError, str]] = []

for error in raw_errors:
path = f'{prefix}{".".join(str(p) for p in error.path)}'

errors.append((error, f'{path} - {error.message}'))

return errors


def validate_fmf_node(
node: fmf.Tree,
schema_name: str,
Expand All @@ -5323,19 +5354,7 @@ def validate_fmf_node(
if result.result is True:
return []

# A bit of error formatting. It is possible to use str(error), but the result
# is a bit too JSON-ish. Let's present an error message in a way that helps
# users to point finger on each and every issue. But don't throw the original
# errors away!

errors: list[tuple[jsonschema.ValidationError, str]] = []

for error in result.errors:
path = f'{node.name}:{".".join(error.path)}'

errors.append((error, f'{path} - {error.message}'))

return errors
return preformat_jsonschema_validation_errors(result.errors, prefix=node.name)


# A type for callbacks given to wait()
Expand Down
Loading