haskell / haskell-language-server

Official haskell ide support via language server (LSP). Successor of ghcide & haskell-ide-engine.
Apache License 2.0
2.61k stars 351 forks source link

Fix flaky test by a test env setup refactor #4273

Open komikat opened 3 weeks ago

komikat commented 3 weeks ago

Fixes #4200 by creating a separate temporary .cache directory for every test instead of having all tests share the same .cache directory. This makes sure the test iface-error-test-1 is run with a clean directory — causing the diagnostic to be generated every time.

soulomoon commented 3 weeks ago

It seems hls-refactor-plugin-tests depend on the cache directory.🤔

/tmp/hls-test-root/.cache-b4d2c0447616dd51: createDirectory: does not exist (No such file or directory)
fendor commented 3 weeks ago

That seems impossible to me, there must be another race condition :D

komikat commented 3 weeks ago

Weird because hls-refactor-plugin-tests seems to pass on my system (MacOS, ghc9.8)

komikat commented 3 weeks ago

The setup wasn't creating the hls-test-root directory. ghcide-tests is still failing, will look into this soon.

komikat commented 1 week ago

definition.Imported symbol (reexported) fails without $TEMPDIR/hls-test-root

running HLS_TEST_HARNESS_NO_TESTDIR_CLEANUP=1 HLS_TEST_LOG_STDERR=1 TASTY_PATTERN="/definition.Imported symbol (reexported)/" cabal test ghcide-tests FAILS (>=GHC94)

expected: Location {_uri = Uri {getUri = "$TMPDIR/hls-test-root/extra-dir-85406152527/Bar.hs"}, _range = Range {_start = Position {_line = 3, _character = 5}, _end = Position {_line = 3, _character = 8}}}
 but got: Location {_uri = Uri {getUri = "$TMPDIR/hls-test-root/extra-dir-85406152527/Bar.hs"}, _range = Range {_start = Position {_line = 3, _character = 0}, _end = Position {_line = 3, _character = 14}}}

Notes

fendor commented 3 days ago

This is good!

Let's merge this as is, add the comment to the test case, create an issue containing the knowledge you have gained so far, and then accept the test case changes. E.g., update the assertions in the test. Even though we have discovered an interesting race (?) condition, the test case itself will be stable.

@komikat Could you do that, please?