Skip to content

fix(checks): improve suite failure check info#2529

Open
harsh21234i wants to merge 9 commits into
Giskard-AI:mainfrom
harsh21234i:fix/suite-result-check-info
Open

fix(checks): improve suite failure check info#2529
harsh21234i wants to merge 9 commits into
Giskard-AI:mainfrom
harsh21234i:fix/suite-result-check-info

Conversation

@harsh21234i

Copy link
Copy Markdown
Contributor

Closes #2521

Summary

  • use check kind as a fallback label when check name is missing
  • capture compact check parameters in result details
  • show check parameters in console and formatted failure reports
  • keep JUnit check labels consistent with console/test case reports

Testing

  • .venv\Scripts\python.exe -m pytest -q libs/giskard-checks/tests/scenarios/test_testcase.py libs/giskard-checks/ tests/export/test_junit.py
  • uv run --no-sync ruff check libs/giskard-checks/src/giskard/checks/core/result.py libs/giskard-checks/src/giskard/ checks/testing/runner.py libs/giskard-checks/src/giskard/checks/export/junit.py libs/giskard-checks/tests/scenarios/ test_testcase.py libs/giskard-checks/tests/export/test_junit.py

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces the extraction and formatting of check parameters to display them in console reports and failure messages. It also refactors check label retrieval by adding a check_label property to CheckResult. The review feedback correctly identifies potential AttributeError risks where details are accessed via .get() without first verifying that the object is a dictionary, and provides defensive coding suggestions to prevent runtime crashes.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread libs/giskard-checks/src/giskard/checks/core/result.py Outdated
Comment thread libs/giskard-checks/src/giskard/checks/core/result.py Outdated
Comment thread libs/giskard-checks/src/giskard/checks/core/result.py Outdated
@harsh21234i

Copy link
Copy Markdown
Contributor Author

Addressed the review comments by making details access defensive in CheckResult.

  • check_label now falls back safely when details is not a dict
  • check_params rendering in console output and formatted failures now guards against non-dict details
  • added a regression test covering CheckResult with non-dict details

Checks run:

  • pytest -q libs/giskard-checks/tests/scenarios/test_testcase.py libs/giskard-checks/tests/export/test_junit.py
  • uv run --no-sync ruff check libs/giskard-checks/src/giskard/checks/core/result.py libs/giskard-checks/src/giskard/ checks/testing/runner.py libs/giskard-checks/src/giskard/checks/export/junit.py libs/giskard-checks/tests/scenarios/ test_testcase.py libs/giskard-checks/tests/export/test_junit.py

@kevinmessiaen kevinmessiaen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

To me this seems not robust. We should probably make the check name a direct property of CheckResult instead of having to guess where it's inside the details

@harsh21234i

Copy link
Copy Markdown
Contributor Author

going through changes ...

@harsh21234i

Copy link
Copy Markdown
Contributor Author

Addressed your's feedback by making check_name a first-class field on CheckResult instead of inferring it from
details.

  • the runner now sets check_name directly from the live Check instance
  • check_label reads the explicit field only
  • legacy details-based naming is still accepted at construction time for backward compatibility
  • updated the tests to use direct check_name values

Also stabilized the suite parallel timing test so it compares parallel vs serial execution directly.

Checks:

  • pytest -q libs/giskard-checks/tests/core/test_suite.py libs/giskard-checks/tests/export/test_junit.py
  • ruff check libs/giskard-checks/src/giskard/checks/core/result.py libs/giskard-checks/src/giskard/checks/testing/ runner.py libs/giskard-checks/src/giskard/checks/scenarios/runner.py libs/giskard-checks/tests/core/test_suite.py libs/giskard-checks/tests/export/test_junit.py

@harsh21234i

Copy link
Copy Markdown
Contributor Author

any update @kevinmessiaen ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

Improve check information returned on Suite Result report

2 participants