ghcide test IfaceTests, we should remove interface cache dir before we run the test, or the diganostic simply gone

[soulomoon]

This can happened in the second round doing the test.

Reproduce

Run this consectively

LSP_TEST_LOG_MESSAGES=1 LSP_TEST_LOG_STDERR=1  TASTY_PATTERN="iface-error-test-1" cabal test ghcide-tests

we are seeing

Debug | SUCCEEDED LOADING HIE FILE FOR /Users/ares/.cache/ghcide/main-67f5d3e80e9a5dab967334c225648bdcd080465f-67f5d3e80e9a5dab967334c225648bdcd080465f/P.hie

The test past again if we remove the cache dir entirely

[soulomoon]

A possible solution to this would be to remove the .cache dir in setupTestEnvironment, after we migrate this test to hls-test-utils.
But I need to see how much it is slowing down the test.

[komikat]

The test seems to time out on re-runs.

Exception: Timed out waiting to receive a message from the server.
Last message received:
{
    "jsonrpc": "2.0",
    "method": "$/progress",
    "params": {
        "token": "11",
        "value": {
            "kind": "end",
            "message": "Finished indexing 3 files"
        }
    }
}

[soulomoon]

@komikat Yes, since if we have the cache, we won't compile it again and hence the diganostic is not genereated

[komikat]

@soulomoon so expected behaviour should be to not wait for a diagnosis? or delete the specific component being cached before the test is run?

[soulomoon]

@komikat The point of the test is to assert wether the diganostic would arrive. So may be the latter.

[komikat]

A possible solution to this would be to remove the .cache dir in setupTestEnvironment, after we migrate this test to hls-test-utils.

I tried removing the cache dir in setupTestEnvironment like this:

setupTestEnvironment :: IO FilePath
setupTestEnvironment = do
  tmpDirRoot <- getTemporaryDirectory
  let testRoot = tmpDirRoot </> "hls-test-root"
      testCacheDir = testRoot </> ".cache"
  dirExists <- doesDirectoryExist testCacheDir
  when dirExists $ removeDirectoryRecursive testCacheDir
  createDirectory testCacheDir
  setEnv "XDG_CACHE_HOME" testCacheDir
  pure testRoot

this solves iface-error-test-1 being flaky but causes other tests to fail.
Maybe we can move to a function which removes the directory for tests like this?

[soulomoon]

this solves iface-error-test-1 being flaky but causes other tests to fail.

Then those tests are very suspicious !(Being not worked without cache) 🤔
as

-- However, it is totally safe to delete the directory between runs.

Something must be wrong in our tests.

[komikat]

Something must be wrong in our tests.

Maybe not!
I took a look at the fails and its this specific exception everytime:

Exception: ($TMPDIR):
removeDirectoryRecursive:removeContentsRecursive:removePathRecursive:removeContentsRecursive:removePathRecursive:getSymbolicLinkStatus: 
does not exist (No such file or directory)

This suggests there being a race condition somewhere causing the directory to be deleted in between these two lines.

dirExists <- doesDirectoryExist testCacheDir
when dirExists $ removeDirectoryRecursive testCacheDir

I tried handling the exception itself instead — this way:

setupTestEnvironment :: IO FilePath
setupTestEnvironment = do
  tmpDirRoot <- getTemporaryDirectory
  let testRoot = tmpDirRoot </> "hls-test-root"
      testCacheDir = testRoot </> ".cache"
  exc <- try $ removeDirectoryRecursive testCacheDir
  case exc of
    Left e ->
      if isDoesNotExistError e
         then return ()
         else ioError e
    Right _ ->
      return ()
  createDirectory testCacheDir
  setEnv "XDG_CACHE_HOME" testCacheDir
  pure testRoot

This gets rid of all previous exceptions(!) but creates a new one:

Exception: $TMPDIR: createDirectory: already exists (File exists)

This is probably due to the same race condition causing the previous issue. We can use createDirectoryIfMissing instead but
I'm not sure why these race conditions exist considering all of the tests are being run sequentially.

Tagging @fendor since I'm not sure if I'm heading in the right direction.

[fendor]

It looks like sharing the .cache with all tests is too coarse, how about we create one .cache dir per test case? createTempDirectory might be helpful. E.g. createTempDirectory testRoot ".cache"

[komikat]

Thanks, that solves the issue. I'm only concerned about there being too many directories being created, would it be better to clean up all the temporary dirs after?

[fendor]

All directories are created in the temporary hls root directory. So I think it is fine.