Pr 658 fix completion tests #668
Conversation
completion/Context.hs used to generate a diagnostic for the unused "x", but no
longer does so without -Wunused-binds.
completion/Completion.hs used to generate a diagnostic for the unused import of
Data.Maybe, but no longer does so without -Wunused-imports.
We could add these flags like:
{-# OPTIONS_GHC -Wunused-binds #-}
But that would force us to update all the hardcoded line numbers in the unit tests.
Instead we just add a redundant id, carefully positioned to avoid disturbing any
hardcoded positions.
Snippets are back in ghcide 0.6.0.
Stop messing around the editing the file and reloading it. We are trying to test code completion, not editing files, and the extra complexity is making it harder to maintain the test.
|
@peterwicksstringfield many thanks for fix again the tests against ghcide-0.6.0. I am gonna include this one in #658 to make progress towards fix ci. We can tackle the third point afterwards. |
|
Opps. There are several other testfiles that are no longer producing diagnostics by default and they require fixing as well. Will push soon. |
|
@peterwicksstringfield mmm, and what about remove waitForDiagnostics after opening the file? have you tried it? i started to remove them in Completion and i think it fixed the timeout |
|
I think we were using diagnostics for waiting to the server to be ready to produce completions. Removing diagnostics from testdata files and replacing waitForDiagnostics with sleep's makes the work too. |
|
Want to note that i dont like to use I've changed the tests in my pr to remove "virtual" diagnostics in test data files and replacing The real solution should wait for some server message telling the client is has ended to load the file (with or without diagnostics) and all features are available. @peterwicksstringfield what do you think about closing this? (all commits are already included in #658) |
|
Fully included in #658 |
|
Sorry, I was suddenly much busier than I expected to be and stopped responding. (The good kind of busy, but still.) Thanks for keeping up the momentum. I agree with closing this PR. It has served its purpose now that 658 is merged. I also agree that waitForDiagnostic is not an ideal approach. |
This PR is based on #658 . See the discussion there. It is an attempt to fix the code completion tests which are failing after the upgrade to ghcide 0.6.0.
This is a WIP, these tests are not reliable. I think they weren't reliable even before ghcide 0.6.0 ... but it seems like they are less reliable now. Not enough time to debug it all today, so I'm dumping what I have here.
There are three problems:
ghcide 0.6.0 reports fewer diagnostics by default that ghcide 0.5.0. This was causing some tests to fail because they expected certain files to produce diagnostics, and those files no longer produced diagnostics. Such failures manifest as timeouts but they aren't really about time. Fixed.
ghcide 0.6.0 has code completion snippets, so the code completion tests need to be updated to account for that. Fixed.
There's a third issue that is making the tests randomly fail. I think it was pre-existing but rare and I've only just now noticed. I might know what's happening but I don't have time to really figure it out right now. (We load a file, make a change to it, request code completions, and then check that the completions we get are the ones we expect. But maybe the code completions are based on the contents of the file from before we made the change, because we aren't doing whatever it is we need to do to get the freshest possible results. I thought the way we were doing it was okay but clearly I'm wrong about something. Or maybe that's not the problem at all and its something totally different.)