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 hlint tests over cpp, extensions and ignore hints #674

Merged
merged 3 commits into from
Dec 16, 2020

Conversation

jneira
Copy link
Member

@jneira jneira commented Dec 15, 2020

* expectNoMoreDiagnostics, adapted from ghcide
* add knownBroken and ignore by ghc version
@jneira
Copy link
Member Author

jneira commented Dec 16, 2020

stack build in circleci is ignoring the expectFailBecause and reports tests as failed, but cabal does not, weird

@jneira
Copy link
Member Author

jneira commented Dec 16, 2020

@bubba maybe you could help me with that 🙂
I want to "force" that a session timeout produces an assertion error, for that i would need to catch the Timeout a exception and call assertFailure in that case

But... Session a has no instance for MonadError (nor other suitable error handling class) so i have to implement:

instance MonadError LSPTest.SessionException LSPTest.Session

Would be any way to avoid that orphan instance? or another way to make stack not mark tests as failed in the presence of session timeouts?

In lsp-test the catchError is done over the underlying ConduitParser type: https://github.com/bubba/lsp-test/blob/cd644f52c5c564403b5f3b0a8652e7f4154f8d6a/src/Language/LSP/Test/Session.hs#L212

@jneira
Copy link
Member Author

jneira commented Dec 16, 2020

@bubba never mind, finally i am gonna catch the exception at runSession level, that is IO a so i can use Control.Exception directly, something like (not tested):

failIfSessionTimeout :: IO a -> IO a
failIfSessionTimeout action = action `catch` errorHandler
    where errorHandler :: Test.SessionException -> IO a
          errorHandler e@(Test.Timeout _) = assertFailure $ show e
          errorHandler e = throwIO e

let's see if it works... 🤔

@jneira
Copy link
Member Author

jneira commented Dec 16, 2020

It made work stack tests in circleci, but why does it work in cabal without handling explicitly SessionException? 🤔

@jneira jneira requested review from alanz and lukel97 December 16, 2020 13:34
Copy link
Collaborator

@alanz alanz left a comment

Choose a reason for hiding this comment

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

I don't see anything obviously a problem

@jneira jneira merged commit ffebc7b into haskell:master Dec 16, 2020
@jneira jneira deleted the hlint-tests branch December 16, 2020 21:39
@lukel97
Copy link
Collaborator

lukel97 commented Dec 17, 2020

@jneira yeah that code snippet is what I would do, catch on the specific Timeout exception. I bet you though the failure is to do with parallelism though/the jobs -j flag. The circleCI jobs might not be telling hspec to run the tests sequentially?

@jneira
Copy link
Member Author

jneira commented Dec 17, 2020

@bubba -j1 is passed to the test suite:

name: Test haskell-language-server func-test suite
# Tasty by default will run all the tests in parallel. Which should
# work ok, but given that these CircleCI runners aren't the beefiest
# machine can cause some flakiness. So pass -j1 to Tasty (NOT Stack) to
# tell it to go slow and steady.
command: stack --stack-yaml=${STACK_FILE} test haskell-language-server:func-test --dump-logs --test-arguments="-j1 --rerun-update" || stack --stack-yaml=${STACK_FILE} test haskell-language-server:func-test --dump-logs --test-arguments="-j1 --rerun" || LSP_TEST_LOG_COLOR=0 LSP_TEST_LOG_MESSAGES=true LSP_TEST_LOG_STDERR=true stack --stack-yaml=${STACK_FILE} test haskell-language-server:func-test --dump-logs --test-arguments="-j1 --rerun"
no_output_timeout: 120m

However, being able to catch the exception in Session monad would be nice, as we could add it in the waitForDiagnostics itself, and all tests would have that behaviour at once. Although it could be added to a common runSession for all of them too, i can think off in more cases where we would want to catch SessionException's.
Could we add a MonadError instance in lsp-test for Session? I guess it could be done delegating tryError and catchError to the undelying ConduitParser data type.

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.

3 participants