semgrep / testo

Test framework for OCaml
ISC License
16 stars 1 forks source link

Require a reason when skipping a test #92

Closed mjambon closed 4 weeks ago

mjambon commented 1 month ago

The point of skipped tests is to prevent flaky or non-portable tests from getting in the way. There's always an expectation that these tests should work in some environment or at some point in the future. It would be beneficial to make these expectations visible. Right now, skipping a test is done with a boolean (~skipped:false). Instead, this should provide the reason why the test is skipped similarly to the expected_outcome type.

Here's a proposal:

type skipped =
  | Not_skipped
  | Skipped of string

val create : ... -> ?skipped:skipped -> ...

or maybe we could use a string option and avoid introducing a new type:

val create : ... -> ?skipped:string -> ...

The latter is a bit inconsistent with expected_outcome. The reason why I created a dedicated type for expected_outcome instead of using a string option is that None sounds negative when in fact it means "OK" or "true". We don't have this problem with skipped. We'd use it as follows:

Testo.create name func ~skipped:"flaky for unknown reasons"