[Aster89]

Ready for review & merge

With this PR we do the transition written in the title.

At least we do so in the func-tests. Why are we doing this, is written in #5054, which links to #5020, so in essence this is part of the effort to update cabal-version.

Original text (so following messages makes sense)

I'm still experimenting.

For now there's two commits:

• the oldest is cosmetic, so disregard it,
• with the newest I've just made a copy of one of ormolu's tests in https://github.com/haskell/haskell-language-server/blob/master/test/functional/Format.hs.
  • This is just an attempt, I'm trying to see if I can run a test using runSessionWithServer instead of runSessionWithConfig.

Running cabal run func-test -- -j32 results¹ in this error:

haskell-language-server
  part 1
    plugin config
      generic plugin config
        custom defaults:                                      OK (0.66s)
        custom defaults and user config on some other plugin: OK (0.45s)
        custom defaults and non overlapping user config:      OK (0.47s)
        custom defaults and overlapping user plugin config:   OK (0.25s)
        custom defaults and non plugin user config:           OK (0.23s)
    generate schema
      vscode-extension-schema:                                OK (0.43s)
      generate-default-config:                                OK (0.45s)
      plugins-custom-config-markdown-reference:               OK (0.42s)
  part 2
    format document
      lsp formatting provider
        respects none:                                        OK (0.50s)
        error message contains stderr output:                 FAIL (0.20s)
          test/functional/Format.hs:68:
          Expected exit code prefix, got: ormolu: Internal Error: ormolu: createProcess: posix_spawnp: does not exist (No such file or directory)
          Use -p '/error message contains stderr output/' to rerun this test only.
        can change on the fly:                                IGNORED
          Ormolu plugin disabled
    behaviour on malformed projects
      Missing module diagnostic:                              OK (0.86s)
      Missing module diagnostic - no matching prefix:         OK (0.47s)
    hie-bios
      loads main-is module:                                   OK (12.37s)
    window/workDoneProgress
      sends indefinite progress notifications:                OK (0.68s)
      eval plugin sends progress reports:                     OK (16.56s)
      ormolu plugin sends progress notifications:             IGNORED
        Ormolu plugin disabled
      fourmolu plugin sends progress notifications:           IGNORED
        Fourmolu plugin disabled

1 out of 18 tests failed (17.23s)

Looking at #3968 I suspect the error above might ahve to do with ormolu as an external process?

---

¹ Most of the times; just once I've seen more tests failing, but maybe that's related to some residual flakiness that I can't easily reproduce, having tried 5 more times.

---

Fixes #5054

---

• I read and followed our AI policy

[Aster89]

I've duplicated the first part of the "can change on the fly" test and run it via runSessionWithServerInTmpDir instead of runSessionWithConfig.

However, to include the following parts of the test, where ormolu is run on the file as edited by fourmolu, I'd need runSessionWithServerInTmpDir to use 2 plugins, but the function it delegates to, runSessionWithTestConfig, only expects 1 plugin (inside TestConfig's testPluginDescriptor field). On the other hand, looking at the implementation of runSessionWithTestConfig, nothing seems to prevent us from changing TestConfig in such a way that it can hold a list of plugins.

@fendor, what do you think?

[fendor]

@Aster89 PluginTestDescriptor b implement Monoid, so plugin1 <> plugin2 works as expected. See plugins/hls-cabal-plugin/test/Utils.hs:70 for an example.

[Aster89]

@Aster89 PluginTestDescriptor b implement Monoid

I had no idea!

Great, so there's also mempty should be enough for the case no plugin is present. Thanks!

---

Actually, @fendor, I understand that IdePlugins a implements Monoid, but PluginTestDescriptor b is instantiated with a different b for oromolu as compared to fourmolu, as they use a different LogEvent for the Recorder:

haskell-language-server/plugins/hls-ormolu-plugin/src/Ide/Plugin/Ormolu.hs

Lines 184 to 190 in cdd7d60

	data LogEvent
	= NoVersion Text
	| StdErr Text
	| LogCompiledInVersion String
	| LogExternalVersion [Int]
	| LogOrmoluCommand [String] FilePath
	deriving (Show)

haskell-language-server/plugins/hls-fourmolu-plugin/src/Ide/Plugin/Fourmolu.hs

Lines 198 to 205 in cdd7d60

	data LogEvent
	= NoVersion Text
	| ConfigPath FilePath
	| NoConfigPath [FilePath]
	| StdErr Text
	| LogCompiledInVersion String
	| LogExternalVersion [Int]
	deriving (Show)

The example you give, instead,

haskell-language-server/plugins/hls-cabal-plugin/test/Utils.hs

Line 70 in cdd7d60

failIfSessionTimeout . runSessionWithServerInTmpDir def (cabalPlugin <> cabalHaskellPlugin) vft

uses these two descriptors,

haskell-language-server/plugins/hls-cabal-plugin/test/Utils.hs

Lines 20 to 24 in cdd7d60

	cabalPlugin :: PluginTestDescriptor Ide.Plugin.Cabal.Log
	cabalPlugin = mkPluginTestDescriptor descriptor "cabal"
	cabalHaskellPlugin :: PluginTestDescriptor Ide.Plugin.Cabal.Log
	cabalHaskellPlugin = mkPluginTestDescriptor haskellInteractionDescriptor "cabal-haskell"

that share the same Log.

What do you suggest?

[Aster89]

Based on https://github.com/haskell/haskell-language-server/actions/runs/41673433111/job/126047852893?pr=5154, using DeriveAnyClass wasn't a great idea.

Besides, I should follow the recommended action at https://github.com/haskell/haskell-language-server/actions/runs/41673433111/job/126047853010?pr=5154

[fendor]

To make these tests resemble their previous behaviour, we should be launching HLS as closely to the final haskell-language-server binary as possible. Thus, I suggest to use idePlugins and avoid the CPP by using requiresOrmoluPlugin and requiresFourmoluPlugin test combinators.

[fendor]

Very good, final nitpicks and then you are good to go :)

Thanks for pushing this through, great improvement!