Add tests for some MorphirRuntimeErrors #662
Conversation
| case e => assertNever(s"Expected FailedCoercion but $e was thrown") | ||
| }, | ||
| testExceptionMultiple("UnknownTypeMismatch Test")("exceptionTests", "decimalHundred", List(Data.String("a"))) { | ||
| case TopLevelError(_, _, _: TypeError.UnknownTypeMismatch) => assertTrue(true) |
There was a problem hiding this comment.
We should expect these tests to fail as we improve error messages - the wrapping structure with TopLevelError is messy and should change. (This isn't something that needs to be fixed in this PR, but we should be aware of this brittleness).
There was a problem hiding this comment.
Two things that are a must change in my opinion is the TopLevelError and any private exceptions like "CodeLocatedError".
And yes, I realize these tests will need to change. The implementation choice should make it relatively easy to revise and add to as the error handling changes though, which is the goal in my mind.
There was a problem hiding this comment.
Agreed on both counts - I want the functionality of TopLevel and CodeLocated, but yeah, they should not appear in the type hierarchy.
| }, | ||
| testExceptionMultiple("UnknownTypeMismatch Test")("exceptionTests", "decimalHundred", List(Data.String("a"))) { | ||
| case TopLevelError(_, _, _: TypeError.UnknownTypeMismatch) => assertTrue(true) | ||
| case TopLevelError(_, _, inner) => assertNever(s"Expected UnsupportedType but $inner was thrown") |
There was a problem hiding this comment.
I think these "Expected" look wrong - shouldn't this be UnknownTypeMismatch not UnsupportedTypeMismatch? Given the brittleness I would not specify the expectation at all ("Unexpected exception type thrown")
There was a problem hiding this comment.
That was likely from a copy paste. And on making it generic, fair point, will update.
| @@ -3448,6 +3469,57 @@ object EvaluatorMDMTests extends MorphirBaseSpec { | |||
| ) | |||
| ) | |||
| ) | |||
| ), | |||
| suite("Morphir Runtime Exception Tests")( | |||
There was a problem hiding this comment.
Are these all the errors you were able to replicate, or is this just a first pass to get the unhappy path machinery set up before flushing out additional errors? (I can help you get a few more runtime errors if that would be helpful, or that can be follow up work)
There was a problem hiding this comment.
This is what I was able to replicate. There are quite a few exceptions that I feel should be getting thrown but end up throwing a different error, like UnknownTypeMismatch when I would think ArgNumberMismatch would be thrown. If you have consistent ways of replicating other ones, then I'd love to get them added.
That said, I think adding additional exceptions can be added in separate PRs as needed.
There was a problem hiding this comment.
One trick you can use is to abuse the type checker:
cast : a -> b
cast x = x
Morphir-elm (tho not elm) will compile that; you can use it to generate a lot of the other errors. Incomplete pattern matches should be similar (elm will reject but morphir-elm will not). We can talk about other tricks to sneak things past the compiler if that's helpful. You're also completely right, some of these throw other errors than we'd like... a lot of that depends on how the specific error is encountered.
Let's schedule some time to chat soon, I've got a lot of thoughts on these in general.
Adds logic to the EvaluatorMDMTests that allows for testing situation where we expect exceptions to get thrown. Also adds some simplified testing to test specific exceptions getting thrown.
An added benefit of this setup is that tests for unhappy path testing should be easier to write going forward (when they expect exceptions to get thrown)