Skip to content

Conversation

@hdgarrood
Copy link
Contributor

I'm currently trying to improve the compiler test suite, and so this is to replace assertPartial in the test prelude. This will probably be useful more generally too, though.

@garyb
Copy link
Member

garyb commented Jul 29, 2015

Could you add a comment to the new function(s) please? Also, shouldn't we be using Eff (err :: EXCEPTION | eff) there rather than Unit -> a really?

@hdgarrood
Copy link
Contributor Author

The idea behind this was, if you have something which is Eff (err :: EXCEPTION | eff) a, then it's easy to check whether it throws an exception, using catchException. On the other hand, there's no obvious way to check whether "pure" code will throw an exception.

@garyb
Copy link
Member

garyb commented Jul 29, 2015

In what situation would you want pure code to throw an exception? 😄

@hdgarrood
Copy link
Contributor Author

To make sure that an unsafe function does what you want it to? Eg assertThrows $ unsafeIndex -1 xs? The specific use case I was thinking of is here: https://github.com/purescript/purescript/blob/master/examples/passing/PartialFunction.purs

@garyb
Copy link
Member

garyb commented Jul 29, 2015

Gotcha, that makes sense. Perhaps we should mention something like that in the comment too then ("this is for testing unsafe functions" or whtever), as I think assertThrows actually can't be used to test Eff-based exceptions with this type signature.

@hdgarrood
Copy link
Contributor Author

👍

@garyb
Copy link
Member

garyb commented Jul 29, 2015

Great, thanks!

Oh, er, one last thing - can you remove the import Control.Monad.Eff.Exception (EXCEPTION())? It's breaking in travis as -assert doesn't depend on -exceptions currently 😉

@garyb
Copy link
Member

garyb commented Jul 29, 2015

👍

garyb added a commit that referenced this pull request Jul 29, 2015
@garyb garyb merged commit 3ede58f into purescript:master Jul 29, 2015
@hdgarrood hdgarrood deleted the assert-throws branch July 30, 2015 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants