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 'testWithApplication'' function #843

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

EduardSergeev
Copy link

@EduardSergeev EduardSergeev commented May 13, 2021

Add testWithApplication' function:

testWithApplication' :: Application -> (Port -> IO a) -> IO a

same as testWithApplication function

testWithApplication :: IO Application -> (Port -> IO a) -> IO a

but accepts Application as a pure argument
The requirement of testWithApplication to pass Application parameter in wrapped in IO monad seems unnecessary strict:
Apparently the only action testWithApplicationSettings (the underlying implementation) does is to unwrap it with:

testWithApplicationSettings :: Settings -> IO Application -> (Port -> IO a) -> IO a
testWithApplicationSettings settings mkApp action = do
  callingThread <- myThreadId
  app <- mkApp
  let wrappedApp request respond =
        app request respond `catch` \ e -> do
  [truncated]

into app which can be done outside of testWithApplication' with:

testWithApplication :: IO Application -> (Port -> IO a) -> IO a
testWithApplication mkApp action =
  mkApp >>= flip testWithApplication' action

Before submitting your PR, check that you've:

After submitting your PR:

  • Update the Changelog.md file with a link to your PR
  • Check that CI passes (or if it fails, for reasons unrelated to your change, like CI timeouts)

Same as `testWithApplication` but accepts `Application` as a pure argument
The requirement of `testWithApplication` to pass `Application` parameter in wrapped in `IO` monad seems unnecessary strict:
Apparently the only action `testWithApplicationSettings` (the underlying implementation) does is to unwrap it with:
```haskell
testWithApplicationSettings :: Settings -> IO Application -> (Port -> IO a) -> IO a
testWithApplicationSettings settings mkApp action = do
  callingThread <- myThreadId
  app <- mkApp
  let wrappedApp request respond =
        app request respond `catch` \ e -> do
  [truncated]
```

into `app` which can be done outside of  `testWithApplication'` with:

```haskell
testWithApplication :: IO Application -> (Port -> IO a) -> IO a
testWithApplication mkApp action =
  mkApp >>= flip testWithApplication' action
```
Copy link
Member

@snoyberg snoyberg left a comment

Choose a reason for hiding this comment

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

I'm ambivalent about this change overall, since either way it's easy to work around (either monadic bind or an additional pure). I'd ask for a different function name though, since a simple tick at the end doesn't express "pure instead of wrapped in IO" to me.

warp/parser.tix Outdated Show resolved Hide resolved
warp/stack.yaml Outdated Show resolved Hide resolved
@EduardSergeev EduardSergeev force-pushed the feature/testWithApplication-prime branch from b3428c3 to 2322225 Compare May 13, 2021 06:44
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