-
-
Notifications
You must be signed in to change notification settings - Fork 454
checks: make suite failure reporting configurable #2418
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -509,6 +509,14 @@ class SuiteResult(BaseResult, frozen=True): | |
| ..., description="List of scenario results" | ||
| ) | ||
| duration_ms: int = Field(..., description="Total execution time in milliseconds") | ||
| max_reported_failures: int | None = Field( | ||
| default=20, | ||
| description=( | ||
| "Maximum number of failed or errored scenarios to show in the rich " | ||
| "report. Use None to show all." | ||
| ), | ||
| ge=1, | ||
| ) | ||
|
|
||
| @computed_field | ||
| @property | ||
|
|
@@ -568,31 +576,36 @@ def __rich_console__( | |
| failures_and_errors = self.failures_and_errors | ||
|
|
||
| if failures_and_errors: | ||
| n_loggable_failures = 20 # TODO: make this configurable | ||
| max_reported_failures = self.max_reported_failures | ||
| reported_failures = ( | ||
| failures_and_errors | ||
| if max_reported_failures is None | ||
| else failures_and_errors[:max_reported_failures] | ||
| ) | ||
|
|
||
| # Details | ||
| yield Rule("FAILURES", characters="=", style="grey") | ||
| for f in failures_and_errors[:n_loggable_failures]: | ||
| for f in reported_failures: | ||
| yield Panel( | ||
| f, | ||
| title=f.scenario_name, | ||
| border_style=f"{STATUS_MAPPING[f.status]['color']} bold", | ||
| ) | ||
| if len(failures_and_errors) > n_loggable_failures: | ||
| yield f" ... and {len(failures_and_errors) - n_loggable_failures} more" | ||
| if len(failures_and_errors) > len(reported_failures): | ||
| yield f" ... and {len(failures_and_errors) - len(reported_failures)} more" | ||
|
|
||
| # Summary | ||
| yield Rule("SUMMARY", characters="=", style="grey") | ||
| for f in failures_and_errors[:n_loggable_failures]: | ||
| for f in reported_failures: | ||
| status = STATUS_MAPPING[f.status] | ||
| yield f"[{status['color']} bold]{f.scenario_name}[/{status['color']} bold]\t[{status['color']}]{f.status.value.upper()}[/{status['color']}]" | ||
| for tc in f.failures_and_errors: | ||
| for c in tc.failures_and_errors: | ||
| yield from ( | ||
| f"\t{line}" for line in c.__rich_console__(console, options) | ||
| ) | ||
| if len(failures_and_errors) > n_loggable_failures: | ||
| yield f" ... and {len(failures_and_errors) - n_loggable_failures} more" | ||
| if len(failures_and_errors) > len(reported_failures): | ||
| yield f" ... and {len(failures_and_errors) - len(reported_failures)} more" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The logic for calculating the number of hidden failures and yielding the 'more' message is duplicated. Extracting this into a variable improves maintainability and readability. if failures_and_errors:
max_reported_failures = self.max_reported_failures
reported_failures = (
failures_and_errors
if max_reported_failures is None
else failures_and_errors[:max_reported_failures]
)
n_hidden = len(failures_and_errors) - len(reported_failures)
# Details
yield Rule("FAILURES", characters="=", style="grey")
for f in reported_failures:
yield Panel(
f,
title=f.scenario_name,
border_style=f"{STATUS_MAPPING[f.status]['color']} bold",
)
if n_hidden > 0:
yield f" ... and {n_hidden} more"
# Summary
yield Rule("SUMMARY", characters="=", style="grey")
for f in reported_failures:
status = STATUS_MAPPING[f.status]
yield f"[{status['color']} bold]{f.scenario_name}[/{status['color']} bold]\t[{status['color']}]{f.status.value.upper()}[/{status['color']}]"
for tc in f.failures_and_errors:
for c in tc.failures_and_errors:
yield from (
f"\t{line}" for line in c.__rich_console__(console, options)
)
if n_hidden > 0:
yield f" ... and {n_hidden} more" |
||
|
|
||
| yield Rule(style="bold blue") | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -60,6 +60,14 @@ class Suite(BaseModel, Generic[InputType, OutputType]): | |
| default=NOT_PROVIDED, | ||
| description="Suite-level target SUT that will override any scenario-level target.", | ||
| ) | ||
| max_reported_failures: int | None = Field( | ||
| default=20, | ||
| description=( | ||
| "Maximum number of failed or errored scenarios to show in suite " | ||
| "reports. Use None to show all." | ||
| ), | ||
| ge=1, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| ) | ||
|
|
||
| def append( | ||
| self, | ||
|
|
@@ -146,6 +154,7 @@ async def run( | |
| suite_result = SuiteResult( | ||
| results=results, | ||
| duration_ms=int((end_time - start_time) * 1000), | ||
| max_reported_failures=self.max_reported_failures, | ||
| ) | ||
|
|
||
| telemetry_capture( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,15 @@ | ||
| import pytest | ||
| from giskard.checks import Equals, Scenario, Suite | ||
| from giskard.checks.core.interaction import Trace | ||
| from giskard.checks.core.result import ( | ||
| CheckResult, | ||
| ScenarioResult, | ||
| SuiteResult, | ||
| ) | ||
| from giskard.checks.core.result import ( | ||
| TestCaseResult as CheckTestCaseResult, | ||
| ) | ||
|
Comment on lines
+3
to
+12
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| from rich.console import Console | ||
|
|
||
|
|
||
| @pytest.fixture | ||
|
|
@@ -150,3 +160,89 @@ async def test_suite_append_chaining(): | |
| assert len(result.results) == 2 | ||
| assert result.results[0].scenario_name == "a" | ||
| assert result.results[1].scenario_name == "b" | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_suite_run_propagates_max_reported_failures(): | ||
| scenario = ( | ||
| Scenario("s1") | ||
| .interact("b", "c") | ||
| .check(Equals(expected_value="b", key="trace.last.outputs")) | ||
| ) | ||
|
|
||
| suite = Suite(name="agg_suite", max_reported_failures=3) | ||
| suite.append(scenario) | ||
|
|
||
| result = await suite.run() | ||
|
|
||
| assert result.max_reported_failures == 3 | ||
|
|
||
|
|
||
| def test_suite_result_rich_console_respects_max_reported_failures(): | ||
| def failed_scenario(name: str) -> ScenarioResult[Trace]: | ||
| return ScenarioResult( | ||
| scenario_name=name, | ||
| steps=[ | ||
| CheckTestCaseResult( | ||
| results=[ | ||
| CheckResult.failure( | ||
| message=f"{name} failed", | ||
| details={"check_name": "ExampleCheck"}, | ||
| ) | ||
| ], | ||
| duration_ms=1, | ||
| ) | ||
| ], | ||
| duration_ms=1, | ||
| final_trace=Trace(interactions=[]), | ||
| ) | ||
|
|
||
| result = SuiteResult( | ||
| results=[failed_scenario("s1"), failed_scenario("s2"), failed_scenario("s3")], | ||
| duration_ms=3, | ||
| max_reported_failures=2, | ||
| ) | ||
| console = Console(record=True, width=120) | ||
|
|
||
| console.print(result) | ||
|
|
||
| output = console.export_text() | ||
| assert "s1" in output | ||
| assert "s2" in output | ||
| assert "s3" not in output | ||
| assert "... and 1 more" in output | ||
|
|
||
|
|
||
| def test_suite_result_rich_console_shows_all_failures_when_unbounded(): | ||
| def failed_scenario(name: str) -> ScenarioResult[Trace]: | ||
| return ScenarioResult( | ||
| scenario_name=name, | ||
| steps=[ | ||
| CheckTestCaseResult( | ||
| results=[ | ||
| CheckResult.failure( | ||
| message=f"{name} failed", | ||
| details={"check_name": "ExampleCheck"}, | ||
| ) | ||
| ], | ||
| duration_ms=1, | ||
| ) | ||
| ], | ||
| duration_ms=1, | ||
| final_trace=Trace(interactions=[]), | ||
| ) | ||
|
|
||
| result = SuiteResult( | ||
| results=[failed_scenario("s1"), failed_scenario("s2"), failed_scenario("s3")], | ||
| duration_ms=3, | ||
| max_reported_failures=None, | ||
| ) | ||
| console = Console(record=True, width=120) | ||
|
|
||
| console.print(result) | ||
|
|
||
| output = console.export_text() | ||
| assert "s1" in output | ||
| assert "s2" in output | ||
| assert "s3" in output | ||
| assert "... and" not in output | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider changing the minimum value to
0. Settingmax_reported_failures=0is a valid way to suppress detailed failure logs while still showing the summary count (e.g., '... and 20 more').