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

Add evalNF #384

Merged
merged 2 commits into from
Jun 17, 2020
Merged

Add evalNF #384

merged 2 commits into from
Jun 17, 2020

Conversation

dcastro
Copy link
Contributor

@dcastro dcastro commented May 30, 2020

Added evalNF as discussed in #383.

A couple of things:

  1. I created a local failException', which behaves exactly like failException except it also prints ━━━ Value could not be evaluated to normal form ━━━. Alternatively, we could simply change the type of failException to take an additional list of lines to be printed.
    -- before
    failException :: (MonadTest m, HasCallStack) => SomeException -> m a
    
    -- after
    failException :: (MonadTest m, HasCallStack) => [String] -> SomeException -> m a
    I suppose since failException is internal this would not be a breaking change, but still inconvenient... Any thoughts on whether we should refactor failException or leave the local bind for failException' in?
  2. It wasn't clear to me how to add new tests for evalNF. I tried searching for tests for the existing eval, but couldn't find any. Should I add any tests and, if so, to which project/module?
  3. I forgot to re-export either and either_ from Hedgehog.Gen in my previous PR, so I did that here, if that's OK. I can also submit this is a separate PR if you'd prefer.

@jacobstanley
Copy link
Member

I'm ok with modifying the existing failException to take an argument given it's internal 👍

@jacobstanley
Copy link
Member

It wasn't clear to me how to add new tests for evalNF. I tried searching for tests for the existing eval, but couldn't find any. Should I add any tests and, if so, to which project/module?

I think we don't have any tests for the evalXXX functions at the moment.

You could add a module to hedgehog-example if you're so inclined. Note that the tests in this project don't fail the build, all of the tests are failing on purpose to demonstrate how to use different features.

I'm cool with having no test though as it's so trivial.

@moodmosaic
Copy link
Member

@jacobstanley @dcastro

I'm ok with modifying the existing failException to take an argument given it's internal 👍

Or even make a new top-level one called failExceptionWith taking a message, following the pattern used in all other xyzWith ones. Then have failException use the failExceptionWith one.

@moodmosaic
Copy link
Member

Yep, basically we use eval in some of the examples via the assert one:

assert :: (MonadTest m, HasCallStack) => Bool -> m ()
assert b = do
ok <- withFrozenCallStack $ eval b
if ok then
success
else
withFrozenCallStack failure

Here's one of those examples:

prop_shrink_limit :: Property
prop_shrink_limit =
withShrinks 0 . property $ do
x <- forAll $ Gen.enum 'a' 'z'
assert $
x == 'z'

@dcastro
Copy link
Contributor Author

dcastro commented Jun 14, 2020

Thanks for the feedback!

I've added failExceptionWith, changed failException to use failExceptionWith, and added a couple of tests to hedgehog-example.

Copy link
Member

@moodmosaic moodmosaic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@moodmosaic moodmosaic merged commit eadcf9e into hedgehogqa:master Jun 17, 2020
@moodmosaic
Copy link
Member

Thank you, @dcastro!

@dcastro dcastro deleted the evalNF branch June 17, 2020 09:50
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