haskell / lsp

Haskell library for the Microsoft Language Server Protocol
360 stars 89 forks source link

lsp-test: Add MonadThrow instance to Session #448

Closed thomasjm closed 1 year ago

thomasjm commented 1 year ago

A MonadThrow instance is useful in the Session monad so that you can make assertions in various test frameworks.

This adds exceptions to the dependencies for lsp-test, but it's a pretty common dependency (already used in lsp).

pepeiborra commented 1 year ago

What kind of assertions do you have in mind? Can you share an example?

thomasjm commented 1 year ago

Pretty much all test frameworks I'm aware of throw exceptions to indicate failures. For example, HUnit:

https://hackage.haskell.org/package/HUnit-1.6.2.0/docs/Test-HUnit-Base.html#v:assertEqual

pepeiborra commented 1 year ago

I'm interested in understanding your use case. Would you be able to share an example?

thomasjm commented 1 year ago

I'm testing an LSP server. Here's an example test:

  it "does document highlight" $ doSession documentHighlightCode $ \filename -> do
    ident <- openDoc filename "haskell"
    let desired = [
          DocumentHighlight (Range (Position 0 0) (Position 0 3)) (Just HkWrite)
          , DocumentHighlight (Range (Position 1 9) (Position 1 12)) (Just HkRead)
          ]
    getHighlights ident (Position 0 1) >>= (`shouldBe` List desired)

The doSession command here runs some setup and ultimately calls runSessionWithConfig with the callback, so the body of this test runs in the Session monad. The shouldBe call is the part that needs MonadThrow to work.

michaelpj commented 1 year ago

Huh, what do we do in HLS currently? I thought we did something like this already? It does seem nice to be able to put the assertions in the session body, though.

pepeiborra commented 1 year ago

We use liftIO, because the hunit assertion combinators that we use are not lifted to MonadThrow. I can see how the lifted version enabled by MonadThrow is nicer, so this change seems Ok

michaelpj commented 1 year ago

Oh, if you wanted to be helpful it looks like there's some stuff in the lsp-test tests that could be cleaned up with this!