GSK-2617 Missing kwargs in a test make the whole suite fail

[kevinmessiaen]

Description

Issue

When running a test suite, we do a validation that all the parameters have been set based on the typing annotation. (type is not Optional and no default value has been provided.

The problem is that the whole test Suite is cancelled if only one test is "misconfigured", furthermore it might happen that a required parameter is actually optional but the typing is wrong.

Solution

The validation now show a warning instead of an exception and run the Suite letting the test fail.

How to reproduce

Here is a small snippet that run a Suite with missing param

import giskard
from giskard.core.test_result import TestResult

@giskard.test()
def my_test(is_pass: bool, unused: int):
  return TestResult(passed=is_pass)

suite = giskard.Suite().add_test(my_test()).add_test(my_test(is_pass=True)).add_test(is_pass=True, unused=1)
result = suite.run()

Related Issue

Type of Change

• 📚 Examples / docs / tutorials / dependencies update
• 🔧 Bug fix (non-breaking change which fixes an issue)
• 🥂 Improvement (non-breaking change which improves an existing feature)
• 🚀 New feature (non-breaking change which adds functionality)
• 💥 Breaking change (fix or feature that would cause existing functionality to change)
• 🔐 Security fix

[linear]

GSK-2617 Missing kwargs in a test make the whole suite fail (Hub)

[gitguardian]

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
Once a secret has been leaked into a git repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.

---

<sup><sub>🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!</sub></sup>

[andreybavt]

there's the same check in to_unittest that should be consistent with run

[andreybavt]

why not keep the same check but convert an exception to a warning?

[kevinmessiaen]

Yes that's right, a warning make a lot more sense especially with live logs now

[kevinmessiaen]

@andreybavt I updated the Suite.run and Suite.to_unittest to now raise a warning in case of missing params.

[vjern]

I agree with the overall solution. +1 for early validation of required params, but it should not blockade the test suite being used, and it's worthwhile to still warn the user about it.

Feel free to commit or disregard my other nitpicks.

[sonarcloud]

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud