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

Specify check severity in test scenario documentation only #404

Open
BenjaminPelletier opened this issue Dec 13, 2023 · 1 comment
Open
Labels
enhancement New feature or request P2 Normal priority

Comments

@BenjaminPelletier
Copy link
Member

Is your feature request related to a problem? Please describe.
Currently, an understanding of the expected overall behavior of a test scenario from the test scenario documentation is missing information regarding what will/should happen when a check fails. This is important because failed checks sometimes/often serve as flow control to, for instance, stop the test scenario early or even stop the test run early. Instead, this information is provided only in Python when check.record_failed is called.

Describe the solution you'd like
We already have the ability to specify the severity of a failed check in documentation, and that documentation information is already available programmatically. We should migrate all check severity information into documentation and remove the ability to specify severity at runtime. This will clarify and simplify test behavior.

Describe alternatives you've considered
We could leave maximum flexibility to specify severity in documentation or in code or override documentation in code. While that would provide maximum developer flexibility, I am unaware of any value we would gain/retain under that approach and we would lose the clarity and simplicity that would accompany specification of check severity in documentation only.

Execution plan
The record_failed method has wide usage throughout the codebase (~260 instances) and currently severity is a required parameter. Therefore, we will probably need to phase this change in over many PRs. I suspect a strategy like the following would be effective:

  1. Make severity an optional parameter to record_failed and have it default to the documented value when not specified (throwing ValueError if not specified via either means)
  2. Remove severity parameter from record_failed calls where severity is specified in documentation
  3. Add severity to check documentation in some subset of the codebase (probably not all at once)
  4. Repeat steps 2-3 until all specifications of the severity parameter of record_failed have been removed
  5. Remove the severity parameter from record_failed
@BenjaminPelletier
Copy link
Member Author

When executing step 2, we should keep any eye out for any record_failed invocations that pass the now-removed participants parameter positionally, as it is possible those invocations may have been missed in #403 and #414

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request P2 Normal priority
Projects
None yet
Development

No branches or pull requests

1 participant