search

fscheck / FsCheck

Random Testing for .NET
https://fscheck.github.io/FsCheck/
BSD 3-Clause "New" or "Revised" License
1.17k stars 156 forks source link

Property labels and Exceptions #598

Closed ordinaryorange closed 2 years ago

ordinaryorange commented 2 years ago

Given the following

Check.Quick (lazy failwith "boom" |> Prop.throws<ArgumentException, _> |> Prop.label "Expected ArgumentException")

I was expecting the test result to list the label and the corresponding exception thrown, as the test has technically failed. (Which would be super useful)

I assume this is the correct use of labels ?

kurtschelfthout commented 2 years ago

Exceptions are tricky to deal with.

throws tries to catch exception of the given type, but since in this case it isn't ArgumentException the exception just goes through. label doesn't even get to run. FsCheck could handle all exceptions in label and add the given label if it sees an exception - but then how does it know the exception is not handled a bit further up the stack (and so is not in fact part of the failure). If you have a suggestion feel free.

ordinaryorange commented 2 years ago

I do see what you are saying. The following thoughts come to mind.

  1. If there is a Prop.throws<'T> assertion at a particular location in the stack, and a bubbling exception arrives that is not of the expected type 'T, isn't that a failure of test expectation ? If the exception is supposed to be handled further up the stack, but instead handled at that point, then that seems to me an incorrectly written test. This idea says it is ok to handle the exception at the label
  2. I have not looked at the internals of how Prop.label works, but I have seen that FsCheck does nicely report nested labels. Could the label at the point where an exception type mismatch occurs be added to the output labels and the exceptionreraiseed ? This idea lets the exception move on, but provides more information in the test output as to an inner failures. The only situation I can think of at the moment for reraiseing is for consumption by other test frameworks that require exceptions. But here a thrown ExceptionTypeMismatchException or similar from FsCheck might be a better idea?
kurtschelfthout commented 2 years ago

Agreed, good call. The linked PR makes your example work - see the test I put in there.

I also realized that as long as Prop.label surrounds a code block that is delayed, exceptions are not an issue, i.e. the label should be applied. If it's not delayed though, then we have no chance because the exception is thrown even before the Prop.label code runs.

kurtschelfthout commented 2 years ago

Released in 2.16.4

ordinaryorange commented 2 years ago

works perfectly! Loving FsCheck, well done.