Skip to content

Using captureKicksDiagnostics to speed up multiple plugin tests #4339

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
Aug 2, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ghcide/src/Development/IDE/Core/OfInterest.hs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import Development.IDE.Graph
import Control.Concurrent.STM.Stats (atomically,
modifyTVar')
import Data.Aeson (toJSON)
import qualified Data.Aeson as Aeson
import qualified Data.ByteString as BS
import Data.Maybe (catMaybes)
import Development.IDE.Core.ProgressReporting
Expand Down
28 changes: 20 additions & 8 deletions ghcide/src/Development/IDE/Core/Shake.hs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@
garbageCollectDirtyKeysOlderThan,
Log(..),
VFSModified(..), getClientConfigAction,
ThreadQueue(..)
ThreadQueue(..),
kickSignal
) where

import Control.Concurrent.Async
Expand Down Expand Up @@ -123,6 +124,9 @@
import Development.IDE.Core.PositionMapping
import Development.IDE.Core.ProgressReporting
import Development.IDE.Core.RuleTypes
import qualified Language.LSP.Protocol.Message as LSP
import qualified Language.LSP.Server as LSP

Check warning on line 128 in ghcide/src/Development/IDE/Core/Shake.hs

View workflow job for this annotation

GitHub Actions / Hlint check run

Warning in module Development.IDE.Core.Shake: Use fewer imports ▫︎ Found: "import qualified Language.LSP.Server as LSP\nimport qualified Language.LSP.Server as LSP\n" ▫︎ Perhaps: "import qualified Language.LSP.Server as LSP\n"

import Development.IDE.Core.Tracing
import Development.IDE.Core.WorkerThread
import Development.IDE.GHC.Compat (NameCache,
Expand Down Expand Up @@ -152,6 +156,7 @@
import qualified Focus
import GHC.Fingerprint
import GHC.Stack (HasCallStack)
import GHC.TypeLits (KnownSymbol)
import HieDb.Types
import Ide.Logger hiding (Priority)
import qualified Ide.Logger as Logger
Expand Down Expand Up @@ -1350,29 +1355,28 @@
let uri' = filePathToUri' fp
let delay = if null newDiags then 0.1 else 0
registerEvent debouncer delay uri' $ withTrace ("report diagnostics " <> fromString (fromNormalizedFilePath fp)) $ \tag -> do
join $ mask_ $ do
lastPublish <- atomicallyNamed "diagnostics - publish" $ STM.focus (Focus.lookupWithDefault [] <* Focus.insert newDiags) uri' publishedDiagnostics
let action = when (lastPublish /= newDiags) $ case lspEnv of
join $ mask_ $ do
lastPublish <- atomicallyNamed "diagnostics - publish" $ STM.focus (Focus.lookupWithDefault [] <* Focus.insert newDiags) uri' publishedDiagnostics
let action = when (lastPublish /= newDiags) $ case lspEnv of
Nothing -> -- Print an LSP event.
logWith recorder Info $ LogDiagsDiffButNoLspEnv (map (fp, ShowDiag,) newDiags)
Just env -> LSP.runLspT env $ do
liftIO $ tag "count" (show $ Prelude.length newDiags)
liftIO $ tag "key" (show k)
LSP.sendNotification SMethod_TextDocumentPublishDiagnostics $
LSP.PublishDiagnosticsParams (fromNormalizedUri uri') (fmap fromIntegral ver) newDiags
return action
return action
where
diagsFromRule :: Diagnostic -> Diagnostic
diagsFromRule c@Diagnostic{_range}
| coerce ideTesting = c & L.relatedInformation ?~
[
DiagnosticRelatedInformation
[ DiagnosticRelatedInformation
(Location
(filePathToUri $ fromNormalizedFilePath fp)
_range
)
(T.pack $ show k)
]
]
| otherwise = c


Expand Down Expand Up @@ -1444,3 +1448,11 @@
EM.mapAccumRWithKey (\acc _k (delta, _) -> let new = addOldDelta delta acc in (new, (delta, acc)))
zeroMapping
(EM.insert ver (mkDelta changes, zeroMapping) mappingForUri)

-- | sends a signal whenever shake session is run/restarted
-- being used in cabal and hlint plugin tests to know when its time
-- to look for file diagnostics
kickSignal :: KnownSymbol s => Bool -> Maybe (LSP.LanguageContextEnv c) -> [NormalizedFilePath] -> Proxy s -> Action ()
kickSignal testing lspEnv files msg = when testing $ liftIO $ mRunLspT lspEnv $
LSP.sendNotification (LSP.SMethod_CustomMethod msg) $
toJSON $ map fromNormalizedFilePath files
2 changes: 2 additions & 0 deletions haskell-language-server.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,8 @@ library hls-hlint-plugin
, unordered-containers
, ghc-lib-parser-ex
, apply-refact
--
, lsp-types

if flag(ghc-lib)
cpp-options: -DGHC_LIB
Expand Down
27 changes: 22 additions & 5 deletions hls-test-utils/src/Test/Hls.hs
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,17 @@ module Test.Hls
WithPriority(..),
Recorder,
Priority(..),
TestConfig(..),
captureKickDiagnostics,
kick,
TestConfig(..)
)
where

import Control.Applicative.Combinators
import Control.Concurrent.Async (async, cancel, wait)
import Control.Concurrent.Extra
import Control.Exception.Safe
import Control.Lens ((^.))
import Control.Lens.Extras (is)
import Control.Monad (guard, unless, void)
import Control.Monad.Extra (forM)
Expand All @@ -80,7 +83,7 @@ import qualified Data.Aeson as A
import Data.ByteString.Lazy (ByteString)
import Data.Default (Default, def)
import qualified Data.Map as M
import Data.Maybe (fromMaybe)
import Data.Maybe (fromMaybe, mapMaybe)
import Data.Proxy (Proxy (Proxy))
import qualified Data.Text as T
import qualified Data.Text.Lazy as TL
Expand Down Expand Up @@ -114,6 +117,7 @@ import Ide.PluginUtils (idePluginsToPluginDes
pluginDescToIdePlugins)
import Ide.Types
import Language.LSP.Protocol.Capabilities
import qualified Language.LSP.Protocol.Lens as L
import Language.LSP.Protocol.Message
import qualified Language.LSP.Protocol.Message as LSP
import Language.LSP.Protocol.Types hiding (Null)
Expand Down Expand Up @@ -231,14 +235,14 @@ goldenWithTestConfig
:: Pretty b
=> TestConfig b
-> TestName
-> FilePath
-> VirtualFileTree
-> FilePath
-> FilePath
-> FilePath
-> (TextDocumentIdentifier -> Session ())
-> TestTree
goldenWithTestConfig config title testDataDir path desc ext act =
goldenGitDiff title (testDataDir </> path <.> desc <.> ext)
goldenWithTestConfig config title tree path desc ext act =
goldenGitDiff title (vftOriginalRoot tree </> path <.> desc <.> ext)
$ runSessionWithTestConfig config $ const
$ TL.encodeUtf8 . TL.fromStrict
<$> do
Expand Down Expand Up @@ -869,6 +873,17 @@ setHlsConfig config = do
-- requests!
skipManyTill anyMessage (void configurationRequest)

captureKickDiagnostics :: Session () -> Session () -> Session [Diagnostic]
captureKickDiagnostics start done = do
_ <- skipManyTill anyMessage start
messages <- manyTill anyMessage done
pure $ concat $ mapMaybe diagnostics messages
where
diagnostics :: FromServerMessage' a -> Maybe [Diagnostic]
diagnostics = \msg -> case msg of
FromServerMess SMethod_TextDocumentPublishDiagnostics diags -> Just (diags ^. L.params . L.diagnostics)
_ -> Nothing

waitForKickDone :: Session ()
waitForKickDone = void $ skipManyTill anyMessage nonTrivialKickDone

Expand All @@ -881,9 +896,11 @@ nonTrivialKickDone = kick (Proxy @"kick/done") >>= guard . not . null
nonTrivialKickStart :: Session ()
nonTrivialKickStart = kick (Proxy @"kick/start") >>= guard . not . null


kick :: KnownSymbol k => Proxy k -> Session [FilePath]
kick proxyMsg = do
NotMess TNotificationMessage{_params} <- customNotification proxyMsg
case fromJSON _params of
Success x -> return x
other -> error $ "Failed to parse kick/done details: " <> show other

4 changes: 4 additions & 0 deletions plugins/hls-cabal-plugin/src/Ide/Plugin/Cabal.hs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import Development.IDE.Core.Shake (restartShakeSessio
import qualified Development.IDE.Core.Shake as Shake
import Development.IDE.Graph (Key, alwaysRerun)
import qualified Development.IDE.Plugin.Completions.Logic as Ghcide
import qualified Development.IDE.Types.Options as Options
import Development.IDE.Types.Shake (toKey)
import qualified Distribution.Fields as Syntax
import qualified Distribution.Parsec.Position as Syntax
Expand Down Expand Up @@ -229,7 +230,10 @@ function invocation.
kick :: Action ()
kick = do
files <- HashMap.keys <$> getCabalFilesOfInterestUntracked
Shake.ShakeExtras{ideTesting = Options.IdeTesting testing, lspEnv} <- Shake.getShakeExtras
Shake.kickSignal testing lspEnv files (Proxy @"kick/start/cabal")
void $ uses Types.ParseCabalFile files
Shake.kickSignal testing lspEnv files (Proxy @"kick/done/cabal")

-- ----------------------------------------------------------------
-- Code Actions
Expand Down
9 changes: 5 additions & 4 deletions plugins/hls-cabal-plugin/test/Main.hs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ codeActionUnitTests =
where
maxCompletions = 100


-- ------------------------ ------------------------------------------------
-- Integration Tests
-- ------------------------------------------------------------------------
Expand All @@ -94,23 +95,23 @@ pluginTests =
[ testGroup
"Diagnostics"
[ runCabalTestCaseSession "Publishes Diagnostics on Error" "" $ do
doc <- openDoc "invalid.cabal" "cabal"
diags <- waitForDiagnosticsFromSource doc "cabal"
_ <- openDoc "invalid.cabal" "cabal"
diags <- cabalCaptureKick
unknownLicenseDiag <- liftIO $ inspectDiagnostic diags ["Unknown SPDX license identifier: 'BSD3'"]
liftIO $ do
length diags @?= 1
unknownLicenseDiag ^. L.range @?= Range (Position 3 24) (Position 4 0)
unknownLicenseDiag ^. L.severity @?= Just DiagnosticSeverity_Error
, runCabalTestCaseSession "Clears diagnostics" "" $ do
doc <- openDoc "invalid.cabal" "cabal"
diags <- waitForDiagnosticsFrom doc
diags <- cabalCaptureKick
unknownLicenseDiag <- liftIO $ inspectDiagnostic diags ["Unknown SPDX license identifier: 'BSD3'"]
liftIO $ do
length diags @?= 1
unknownLicenseDiag ^. L.range @?= Range (Position 3 24) (Position 4 0)
unknownLicenseDiag ^. L.severity @?= Just DiagnosticSeverity_Error
_ <- applyEdit doc $ TextEdit (Range (Position 3 20) (Position 4 0)) "BSD-3-Clause\n"
newDiags <- waitForDiagnosticsFrom doc
newDiags <- cabalCaptureKick
liftIO $ newDiags @?= []
, runCabalTestCaseSession "No Diagnostics in .hs files from valid .cabal file" "simple-cabal" $ do
hsDoc <- openDoc "A.hs" "haskell"
Expand Down
15 changes: 15 additions & 0 deletions plugins/hls-cabal-plugin/test/Utils.hs
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
{-# LANGUAGE DataKinds #-}
{-# LANGUAGE DisambiguateRecordFields #-}
{-# LANGUAGE OverloadedStrings #-}

module Utils where

import Control.Monad (guard)
import Data.List (sort)
import Data.Proxy (Proxy (Proxy))
import qualified Data.Text as T
import Ide.Plugin.Cabal (descriptor)
import qualified Ide.Plugin.Cabal
Expand Down Expand Up @@ -52,6 +55,18 @@ runCabalGoldenSession title subdir fp act = goldenWithCabalDoc def cabalPlugin t
testDataDir :: FilePath
testDataDir = "plugins" </> "hls-cabal-plugin" </> "test" </> "testdata"

-- | these functions are used to detect cabal kicks
-- and look at diagnostics for cabal files
-- kicks are run everytime there is a shake session run/restart
cabalKickDone :: Session ()
cabalKickDone = kick (Proxy @"kick/done/cabal") >>= guard . not . null

cabalKickStart :: Session ()
cabalKickStart = kick (Proxy @"kick/start/cabal") >>= guard . not . null

cabalCaptureKick :: Session [Diagnostic]
cabalCaptureKick = captureKickDiagnostics cabalKickStart cabalKickDone

-- | list comparison where the order in the list is irrelevant
(@?==) :: (HasCallStack, Ord a, Show a) => [a] -> [a] -> Assertion
(@?==) l1 l2 = sort l1 @?= sort l2
23 changes: 14 additions & 9 deletions plugins/hls-hlint-plugin/src/Ide/Plugin/Hlint.hs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,9 @@ import System.Environment (setEnv,
unsetEnv)
#endif
import Development.IDE.Core.PluginUtils as PluginUtils
import qualified Development.IDE.Types.Options as Options
import Text.Regex.TDFA.Text ()

-- ---------------------------------------------------------------------

data Log
Expand All @@ -134,7 +136,7 @@ instance Pretty Log where
LogShake log -> pretty log
LogApplying fp res -> "Applying hint(s) for" <+> viaShow fp <> ":" <+> viaShow res
LogGeneratedIdeas fp ideas -> "Generated hlint ideas for for" <+> viaShow fp <> ":" <+> viaShow ideas
LogUsingExtensions fp exts -> "Using extensions for " <+> viaShow fp <> ":" <+> pretty exts
LogUsingExtensions fp exts -> "Using extensions for " <+> viaShow fp <> ":" <> line <> indent 4 (pretty exts)
LogGetIdeas fp -> "Getting hlint ideas for " <+> viaShow fp
LogResolve msg -> pretty msg

Expand Down Expand Up @@ -183,12 +185,12 @@ instance NFData GetHlintDiagnostics
type instance RuleResult GetHlintDiagnostics = ()

-- | Hlint rules to generate file diagnostics based on hlint hints
-- | This rule is recomputed when:
-- | - A file has been edited via
-- | - `getIdeas` -> `getParsedModule` in any case
-- | - `getIdeas` -> `getFileContents` if the hls ghc does not match the hlint default ghc
-- | - The client settings have changed, to honour the `hlintOn` setting, via `getClientConfigAction`
-- | - The hlint specific settings have changed, via `getHlintSettingsRule`
-- This rule is recomputed when:
-- - A file has been edited via
-- - `getIdeas` -> `getParsedModule` in any case
-- - `getIdeas` -> `getFileContents` if the hls ghc does not match the hlint default ghc
-- - The client settings have changed, to honour the `hlintOn` setting, via `getClientConfigAction`
-- - The hlint specific settings have changed, via `getHlintSettingsRule`
rules :: Recorder (WithPriority Log) -> PluginId -> Rules ()
rules recorder plugin = do
define (cmapWithPrio LogShake recorder) $ \GetHlintDiagnostics file -> do
Expand All @@ -202,8 +204,11 @@ rules recorder plugin = do
liftIO $ argsSettings flags

action $ do
files <- getFilesOfInterestUntracked
void $ uses GetHlintDiagnostics $ Map.keys files
files <- Map.keys <$> getFilesOfInterestUntracked
Shake.ShakeExtras{ideTesting = Options.IdeTesting testing, lspEnv} <- Shake.getShakeExtras
Shake.kickSignal testing lspEnv files (Proxy @"kick/start/hlint")
void $ uses GetHlintDiagnostics files
Shake.kickSignal testing lspEnv files (Proxy @"kick/done/hlint")

where

Expand Down
Loading
Loading