-
Notifications
You must be signed in to change notification settings - Fork 258
feat/damlc test improved output #23139
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
base: main
Are you sure you want to change the base?
Changes from all commits
32bb39f
b854e4d
6df7f02
842d49e
45ac501
8857188
78e9b88
5dcd752
66684f8
8586324
788c0de
0af9da5
679e7e3
7017e5b
31fb9a6
1438dcb
291f279
a10bb25
706f343
50815df
04860b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -41,7 +41,6 @@ import qualified Data.Text as T | |||||||||
| import qualified Data.Text.IO as TIO | ||||||||||
| import qualified Data.Text.Lazy as TL | ||||||||||
| import Data.Tuple.Extra | ||||||||||
| import qualified Data.Vector as V | ||||||||||
| import Development.IDE.Core.API | ||||||||||
| import Development.IDE.Core.IdeState.Daml | ||||||||||
| import Development.IDE.Core.RuleTypes.Daml | ||||||||||
|
|
@@ -52,11 +51,11 @@ import qualified Development.Shake as Shake | |||||||||
| import Safe | ||||||||||
| import qualified ScriptService as SS | ||||||||||
| import qualified DA.Cli.Damlc.Test.TestResults as TR | ||||||||||
| import System.Console.ANSI (SGR(..), setSGRCode, Underlining(..), ConsoleIntensity(..)) | ||||||||||
| import System.Console.ANSI (SGR(..), setSGRCode, Underlining(..), ConsoleIntensity(..), Color(..), ColorIntensity(..), ConsoleLayer(..)) | ||||||||||
| import System.Directory (createDirectoryIfMissing) | ||||||||||
| import System.Exit (exitFailure) | ||||||||||
| import System.FilePath | ||||||||||
| import System.IO (hPutStrLn, stderr) | ||||||||||
| import System.IO (hIsTerminalDevice, hPutStrLn, stderr, stdout) | ||||||||||
| import System.IO.Error (isPermissionError, isAlreadyExistsError, isDoesNotExistError) | ||||||||||
| import qualified Text.XML.Light as XML | ||||||||||
| import qualified Text.Blaze.Html.Renderer.Text as Blaze | ||||||||||
|
|
@@ -67,6 +66,8 @@ import ComponentVersion.Class (ComponentVersioned) | |||||||||
|
|
||||||||||
| newtype UseColor = UseColor {getUseColor :: Bool} | ||||||||||
| newtype ShowCoverage = ShowCoverage {getShowCoverage :: Bool} | ||||||||||
|
|
||||||||||
| type ScriptTestResult = (TR.LocalOrExternal, ScriptName, Either SSC.Error SSC.ScriptResult) | ||||||||||
| newtype CoverageFilter = CoverageFilter {getCoverageFilter :: Regex} | ||||||||||
| newtype RunAllOption = RunAllOption {getRunAllTests :: Bool} | ||||||||||
| newtype TableOutputPath = TableOutputPath {getTableOutputPath :: Maybe String} | ||||||||||
|
|
@@ -91,16 +92,27 @@ execTest | |||||||||
| -> TransactionsOutputPath | ||||||||||
| -> CoveragePaths | ||||||||||
| -> [CoverageFilter] | ||||||||||
| -> Maybe FilePath | ||||||||||
| -> IO () | ||||||||||
| execTest inFiles runAllOption coverage color mbJUnitOutput mPkgConfig opts tableOutputPath transactionsOutputPath resultsIO coverageFilters = do | ||||||||||
| execTest inFiles runAllOption coverage color mbJUnitOutput mPkgConfig opts tableOutputPath transactionsOutputPath resultsIO coverageFilters mbProjectPath = do | ||||||||||
| loggerH <- getLogger opts "test" | ||||||||||
| color <- if getUseColor color then pure color else do | ||||||||||
| isTTY <- hIsTerminalDevice stdout | ||||||||||
| pure $ UseColor isTTY | ||||||||||
| let optsWithPkg = case mPkgConfig of | ||||||||||
| Just PackageConfigFields{..} -> opts { optMbPackageName = Just pName, optMbPackageVersion = pVersion } | ||||||||||
| Nothing -> opts | ||||||||||
| -- Prefer package name over path for identification in headers | ||||||||||
| packageIdentifier = case mPkgConfig of | ||||||||||
| Just PackageConfigFields{..} -> Just $ T.unpack $ LF.unPackageName pName | ||||||||||
| Nothing -> mbProjectPath | ||||||||||
| printTestSuiteBegin color packageIdentifier | ||||||||||
| withDamlIdeState optsWithPkg loggerH noopLogger $ \h -> do | ||||||||||
| flip finally (getDiagnostics h >>= printDiagnostics stderr) $ do | ||||||||||
| runAndReport h inFiles (optDetailLevel opts) (optDamlLfVersion opts) runAllOption coverage color mbJUnitOutput tableOutputPath transactionsOutputPath resultsIO coverageFilters | ||||||||||
| summaryResults <- runAndReport h inFiles (optDetailLevel opts) (optDamlLfVersion opts) runAllOption coverage mbJUnitOutput tableOutputPath transactionsOutputPath resultsIO coverageFilters | ||||||||||
| diags <- getDiagnostics h | ||||||||||
| -- Pass both identifier (for header) and path (for relativizing test file paths) | ||||||||||
| printSummary color packageIdentifier mbProjectPath summaryResults | ||||||||||
| when (any (\(_, _, diag) -> Just DsError == _severity diag) diags) exitFailure | ||||||||||
|
|
||||||||||
| loadAggregatePrintResults :: CoveragePaths -> [CoverageFilter] -> ShowCoverage -> Maybe TR.TestResults -> IO () | ||||||||||
|
|
@@ -131,19 +143,17 @@ runAndReport :: | |||||||||
| -> LF.Version | ||||||||||
| -> RunAllOption | ||||||||||
| -> ShowCoverage | ||||||||||
| -> UseColor | ||||||||||
| -> Maybe FilePath | ||||||||||
| -> TableOutputPath | ||||||||||
| -> TransactionsOutputPath | ||||||||||
| -> CoveragePaths | ||||||||||
| -> [CoverageFilter] | ||||||||||
| -> IO () | ||||||||||
| runAndReport ideState inFiles lvl lfVersion runAllOption coverage color mbJUnitOutput tableOutputPath transactionsOutputPath resultsIO coverageFilters = do | ||||||||||
| -> IO [ScriptTestResult] | ||||||||||
| runAndReport ideState inFiles lvl lfVersion runAllOption coverage mbJUnitOutput tableOutputPath transactionsOutputPath resultsIO coverageFilters = do | ||||||||||
| (localResults, extResults) <- runAllScripts ideState inFiles runAllOption | ||||||||||
| let allResults = localResults ++ extResults | ||||||||||
| let allPackages = [loe | TR.ScriptResults loe _ _ <- allResults] | ||||||||||
| -- print test summary after all tests have run | ||||||||||
| printSummary color [(loe, scriptName, res) | TR.ScriptResults loe _ (Just results) <- allResults, (scriptName, res) <- results] | ||||||||||
| let summaryResults = [(loe, scriptName, res) | TR.ScriptResults loe _ (Just results) <- allResults, (scriptName, res) <- results] | ||||||||||
|
|
||||||||||
| let newTestResults = TR.scriptResultsToTestResults allPackages allResults | ||||||||||
| loadAggregatePrintResults resultsIO coverageFilters coverage (Just newTestResults) | ||||||||||
|
|
@@ -179,6 +189,8 @@ runAndReport ideState inFiles lvl lfVersion runAllOption coverage color mbJUnitO | |||||||||
| ] | ||||||||||
| writeFile junitOutput $ XML.showTopElement $ toJUnit res | ||||||||||
|
|
||||||||||
| pure summaryResults | ||||||||||
|
|
||||||||||
| runAllScripts :: IdeState -> [NormalizedFilePath] -> RunAllOption -> IO ([TR.ScriptResults], [TR.ScriptResults]) | ||||||||||
| runAllScripts h inFiles (RunAllOption runAllOption) = do | ||||||||||
| -- make sure none of the files disappear | ||||||||||
|
|
@@ -295,25 +307,98 @@ failedTestOutput h file = do | |||||||||
| pure $ map (, Just errMsg) scriptNames | ||||||||||
|
|
||||||||||
|
|
||||||||||
| printSummary :: UseColor -> [(TR.LocalOrExternal, ScriptName, Either SSC.Error SSC.ScriptResult)] -> IO () | ||||||||||
| printSummary color res = | ||||||||||
| printTestSuiteBegin :: UseColor -> Maybe String -> IO () | ||||||||||
| printTestSuiteBegin color mbIdentifier = | ||||||||||
| whenJust mbIdentifier $ \identifier -> do | ||||||||||
| let colored = getUseColor color | ||||||||||
| putStrLn $ | ||||||||||
| (if colored then setSGRCode [SetConsoleIntensity BoldIntensity] else "") | ||||||||||
| <> "Running tests (" <> identifier <> ") ..." | ||||||||||
| <> (if colored then setSGRCode [] else "") | ||||||||||
|
Comment on lines
+315
to
+317
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead, let's try a function like "renderColor"
Suggested change
Which handles unsetting the color, and lets us add another backend in the future (Html, for IDE output) |
||||||||||
|
|
||||||||||
| printSummary :: UseColor -> Maybe String -> Maybe FilePath -> [ScriptTestResult] -> IO () | ||||||||||
| printSummary color mbIdentifier mbProjectPath res = | ||||||||||
| liftIO $ do | ||||||||||
| putStrLn $ | ||||||||||
| unlines | ||||||||||
| [ setSGRCode [SetUnderlining SingleUnderline, SetConsoleIntensity BoldIntensity] | ||||||||||
| , "Test Summary" <> setSGRCode [] | ||||||||||
| ] | ||||||||||
| printScriptResults color res | ||||||||||
|
|
||||||||||
| printScriptResults :: UseColor -> [(TR.LocalOrExternal, ScriptName, Either SSC.Error SS.ScriptResult)] -> IO () | ||||||||||
| printScriptResults color results = do | ||||||||||
| liftIO $ forM_ results $ \(loe, ScriptName scriptName, resultOrErr) -> do | ||||||||||
| let name = DA.Pretty.pretty (TR.localOrExternalName loe) <> ":" <> DA.Pretty.pretty scriptName | ||||||||||
| let stringStyleToRender = if getUseColor color then DA.Pretty.renderColored else DA.Pretty.renderPlain | ||||||||||
| putStrLn $ stringStyleToRender $ | ||||||||||
| case resultOrErr of | ||||||||||
| Left _err -> name <> ": " <> DA.Pretty.error_ "failed" | ||||||||||
| Right result -> name <> ": " <> prettyResult result | ||||||||||
| let failedTests = [r | r@(_, _, Left _) <- res] | ||||||||||
| nFailed = length failedTests | ||||||||||
| nPassed = length res - nFailed | ||||||||||
| nTotal = length res | ||||||||||
| colored = getUseColor color | ||||||||||
| identifierSuffix = maybe "" (\ident -> " (" <> ident <> ")") mbIdentifier | ||||||||||
|
|
||||||||||
| -- Handle the "no tests found" case | ||||||||||
| if nTotal == 0 | ||||||||||
| then do | ||||||||||
| putStrLn $ | ||||||||||
| (if colored then setSGRCode [SetUnderlining SingleUnderline, SetConsoleIntensity BoldIntensity] else "") | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here |
||||||||||
| <> "Test Summary" <> identifierSuffix | ||||||||||
| <> (if colored then setSGRCode [] else "") | ||||||||||
| <> ": No tests found" | ||||||||||
| else do | ||||||||||
| let countLine | ||||||||||
| | nFailed > 0 = | ||||||||||
| (if colored then setSGRCode [SetColor Foreground Vivid Red] else "") | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here |
||||||||||
| <> show nFailed <> " failed" | ||||||||||
| <> (if colored then setSGRCode [] else "") | ||||||||||
| <> ", " <> show nPassed <> " passed" | ||||||||||
| | otherwise = | ||||||||||
| (if colored then setSGRCode [SetColor Foreground Vivid Green] else "") | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here, and I let you find the rest |
||||||||||
| <> show nPassed <> " passed" | ||||||||||
| <> (if colored then setSGRCode [] else "") | ||||||||||
|
|
||||||||||
| -- Print combined header line | ||||||||||
| putStrLn $ | ||||||||||
| (if colored then setSGRCode [SetUnderlining SingleUnderline, SetConsoleIntensity BoldIntensity] else "") | ||||||||||
| <> "Test Summary" <> identifierSuffix | ||||||||||
| <> (if colored then setSGRCode [] else "") | ||||||||||
| <> ": " <> countLine | ||||||||||
|
|
||||||||||
| -- Only show failed tests in summary (passed tests are hidden when there are failures) | ||||||||||
| -- This keeps the output focused on what needs attention | ||||||||||
| when (nFailed == 0) $ | ||||||||||
| printScriptResults color mbProjectPath res | ||||||||||
|
|
||||||||||
| -- Show failed tests last (most visible) | ||||||||||
| when (nFailed > 0) $ | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make sure to report that these are only the failed tests, so that it's clear that the successes aren't included. |
||||||||||
| printScriptResults color mbProjectPath failedTests | ||||||||||
|
|
||||||||||
| printScriptResults :: UseColor -> Maybe FilePath -> [ScriptTestResult] -> IO () | ||||||||||
| printScriptResults color mbProjectPath results = do | ||||||||||
| -- Group results by file/package | ||||||||||
| let grouped = groupBy (\(loe1, _, _) (loe2, _, _) -> TR.localOrExternalName loe1 == TR.localOrExternalName loe2) results | ||||||||||
| liftIO $ forM_ grouped $ \groupResults -> do | ||||||||||
| -- Use short relative paths in test summary for readability. | ||||||||||
| -- Note: makeRelative returns the absolute path unchanged if paths have different | ||||||||||
| -- roots (e.g., different drives on Windows). This is acceptable as the absolute | ||||||||||
| -- path is still a valid, clickable path in the output. | ||||||||||
| let (loe, _, _) = head groupResults | ||||||||||
| loeName <- case loe of | ||||||||||
| TR.Local file _ -> do | ||||||||||
| let absPath = fromNormalizedFilePath file | ||||||||||
| let relativePath = maybe absPath (\projectPath -> makeRelative projectPath absPath) mbProjectPath | ||||||||||
| pure $ T.pack relativePath | ||||||||||
| TR.External _ -> pure $ TR.localOrExternalName loe | ||||||||||
| let colored = getUseColor color | ||||||||||
| failedResults = [(name, err) | (_, ScriptName name, Left err) <- groupResults] | ||||||||||
| passedResults = [(name, res) | (_, ScriptName name, Right res) <- groupResults] | ||||||||||
| nFailed = length failedResults | ||||||||||
| nPassed = length passedResults | ||||||||||
| -- Print summary line for this file | ||||||||||
| if nFailed > 0 | ||||||||||
| then do | ||||||||||
| let failedNames = map fst failedResults | ||||||||||
| testWord = if nFailed == 1 then "test" else "tests" | ||||||||||
| failedText = if colored then setSGRCode [SetColor Foreground Vivid Red] <> "failed" <> setSGRCode [] else "failed" | ||||||||||
| if nFailed == 1 | ||||||||||
| then putStrLn $ T.unpack loeName <> ": 1 " <> testWord <> " " <> failedText <> ": " <> T.unpack (head failedNames) | ||||||||||
| else do | ||||||||||
| putStrLn $ T.unpack loeName <> ": " <> show nFailed <> " " <> testWord <> " " <> failedText | ||||||||||
| forM_ failedNames $ \name -> | ||||||||||
| putStrLn $ " - " <> T.unpack name | ||||||||||
| else do | ||||||||||
| let testWord = if nPassed == 1 then "test" else "tests" | ||||||||||
| passedText = if colored then setSGRCode [SetColor Foreground Vivid Green] <> "passed" <> setSGRCode [] else "passed" | ||||||||||
| putStrLn $ T.unpack loeName <> ": " <> show nPassed <> " " <> testWord <> " " <> passedText | ||||||||||
|
|
||||||||||
|
|
||||||||||
| prettyErr :: PrettyLevel -> LF.Version -> SSC.Error -> DA.Pretty.Doc Pretty.SyntaxClass | ||||||||||
|
|
@@ -328,15 +413,6 @@ prettyErr lvl lfVersion err = case err of | |||||||||
| SSC.ExceptionError e -> DA.Pretty.string $ show e | ||||||||||
|
|
||||||||||
|
|
||||||||||
| prettyResult :: SS.ScriptResult -> DA.Pretty.Doc Pretty.SyntaxClass | ||||||||||
| prettyResult result = | ||||||||||
| let nTx = length (SS.scriptResultScriptSteps result) | ||||||||||
| nActive = length $ filter (SS.isActive (SS.activeContractsFromScriptResult result)) (V.toList (SS.scriptResultNodes result)) | ||||||||||
| in DA.Pretty.typeDoc_ "ok, " | ||||||||||
| <> DA.Pretty.int nActive <> DA.Pretty.typeDoc_ " active contracts, " | ||||||||||
| <> DA.Pretty.int nTx <> DA.Pretty.typeDoc_ " transactions." | ||||||||||
|
|
||||||||||
|
|
||||||||||
| toJUnit :: [(NormalizedFilePath, [(ScriptName, Maybe T.Text)])] -> XML.Element | ||||||||||
| toJUnit results = | ||||||||||
| XML.node | ||||||||||
|
|
||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dropping unnecessary use of
do