[Migrate BootTests] part of #4173 Migrate ghcide tests to hls test utils #4227
Conversation
| @@ -676,6 +677,25 @@ runSessionWithServer' disableKick pluginsDp conf sconf caps root s = withLock l | |||
| putStrLn $ "Finishing canceling (took " <> showDuration t <> "s)" | |||
| pure x | |||
|
|
|||
| -- | Host a server, and run a test session on it | |||
| -- Note: cwd will be shifted into @root@ in @Session a@ | |||
There was a problem hiding this comment.
I do not think the original is right?
There was a problem hiding this comment.
What do you mean? Looks correct to me. If you don't lock, I don't think the tests will work though, as shifting the cwd will affect all tests.
There was a problem hiding this comment.
Oh I see, the shift happens in the code of lsp package, right?
We might want one with no lock too, so that it can be run inside the one with the lock. Some test in ghcide need to nest the runs.
There was a problem hiding this comment.
No, the shift happens in the executable https://github.com/haskell/haskell-language-server/blob/master/src/Ide/Main.hs#L126
Ok, I understand that nesting might be necessary sometimes. However, the comment should make it clear that all integration tests need to be locked, since we launch a thread, not a new process, for the tests, which all share the same CWD variable.
There was a problem hiding this comment.
Thanks for the classification, it probably be a good idea to eliminate the need for getCurrentDirectory inside IDEMain.defaultMain now. I'll try to do it. Also makeAbsolute.
There was a problem hiding this comment.
EMM, it might be hard, rule have no access to root dir, but just currentworking dir
https://github.com/soulomoon/haskell-language-server/blob/542ea2603f361ba43b839fee0779d1d1ac034c2b/ghcide/src/Development/IDE/Core/Rules.hs#L722
There was a problem hiding this comment.
Also we might not find the file in TemplateHaskell here ? If current dir is not set.
https://github.com/soulomoon/haskell-language-server/blob/67438ef6aa23ad404af2f142e52dce78b617f95c/ghcide/test/exe/DependentFileTest.hs#L40
There was a problem hiding this comment.
Attempt of getting rid of cwd dependent moved to #4231
Focusing on BootTests migration now.
soulomoon
force-pushed
the
soulomoon/update-ghcide-tests-hls-test-utils-BootTests
branch
from
67438ef
to
25108f4
Compare
No description provided.