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

Introducing "should not", "should not satisfy", "should not contain" and "should not return". #8

Closed
wants to merge 8 commits into from

Conversation

shoooe
Copy link

@shoooe shoooe commented Jun 28, 2014

This change introduces shouldNotBe, shouldNotContain, shouldNotSatisfy and shouldNotReturn as well as the corresponding tests.

The changes compiles, but the tests were not run.

Edit: Testing passes.

@sol
Copy link
Member

sol commented Jun 30, 2014

After thinking it over again and considering that we now add 4 new combinators to the API, I'm somewhat reluctant to apply this right away. My main argument is still that examples that use shouldBe are more suitable as documentation and for me documentation is an important aspect of testing.

Last week @fmap asked me why we don't have a shouldNotBe and I somehow convinced him that it's not needed. After I asked him again today, he tells me that he by now thinks that

'shouldNotBe` should not be

(@fmap, please elaborate ;) )

Hspec is here to serve the user, so if the community at large feels the need for those combinators, I'm happy to add them. Still, everything that we add to the API adds a cost for the user + I never had the need for them (or maybe so rarely that I was happy to use shouldSatisfy instead).

Going forward, I see the following options:

  1. Add them to Test.Hspec.Expectations.Contrib for now
  2. Try to rethink hspec-expectations from scratch, making things more composable, so that it's up to the user what he wants to use

I think esp. (2) would be something interesting, even though I have no idea in what direction that would be going. But we could of course do both (1) and (2).

@shoooe
Copy link
Author

shoooe commented Jun 30, 2014

Still, everything that we add to the API adds a cost for the user.

Could you please elaborate on this?

@shoooe
Copy link
Author

shoooe commented Jun 30, 2014

I've tested the changes and they pass (by the way).

@sol
Copy link
Member

sol commented Jul 1, 2014

Still, everything that we add to the API adds a cost for the user.

Could you please elaborate on this?

A user has to look at the API documentation and figure out what he wants to use. Ideally, we would have a small API that is very composable.

All those combinators can be trivially expressed with shouldSatisfy.

Maybe we just want to add shouldNotSatisfy, so that

23 `shouldSatisfy` (not . (== 42))

becomes

23 `shouldNotSatisfy` (== 42)

?

I'll sum up the discussion so far in a new issue and ask on IRC and Twitter for feedback.

@sol sol mentioned this pull request Jul 1, 2014
@shoooe shoooe closed this Jul 1, 2014
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.

None yet

3 participants