testsuite: Supress stderr when running integration tests #10837
Conversation
| @@ -118,6 +123,23 @@ main = do | |||
| (tests config) | |||
| ) | |||
|
|
|||
| -- Tests are run silently, unless they fail. Firstly because it is annoying to | |||
| -- see lots of stderr from your unit tests. Secondly becaus this output | |||
There was a problem hiding this comment.
| -- see lots of stderr from your unit tests. Secondly becaus this output | |
| -- see lots of stderr from your unit tests. Secondly because this output |
| -- see lots of stderr from your unit tests. Secondly becaus this output | ||
| -- leaks into the result of github actions (#8419) | ||
| silentTest :: TestTree -> TestTree | ||
| silentTest = wrapTest silentHelper |
There was a problem hiding this comment.
Note that wrapTest does quite a few things internally. Another possibility (and not incurring a dependency on tasty-expected-failure) would be something like
newtype SilentTest a = SilentTest a
instance IsTest a => IsTest (SilentTest a) where
run opts (SilentTest t) yieldProgress = do
(out, res) <- hCapture [stderr] (run opts t yieldProgress)
let res' = if resultSuccessful res then res else res { resultDetailsPrinter = \indent printer -> printer $ map (replicate indent ' ' ++) (lines out) }
pure res' There was a problem hiding this comment.
I think I prefer to use wrapTest, since it is higher level, and other parts of the testsuite already depend on tasty-expected-failure.
It's a very useful suggestion though, I didn't realise how simple it would have been to roll my own solution.
| silentTest = wrapTest silentHelper | ||
| where | ||
| silentHelper t = do | ||
| (out, res) <- hCapture [stderr] t |
There was a problem hiding this comment.
Could this capture stderr from other tests executed concurrently? Not a big deal obviously, but might be worth to add a comment or enforce single-threaded execution (if not enforced already).
There was a problem hiding this comment.
A good point, I tested with -j8 without this patch and some tests fail anyway, so I will force this testsuite to run sequentially.
This fixes a problem where test output was leaking into GitHub Actions results (#8419), making the output unnecessarily verbose and harder to read. This makes the test output much cleaner while still preserving all the diagnostic information when tests fail. Also force the test to run sequentially, they fail if you try to run them concurrently. I think that there are probably issues to do with setting the working directory when using cabal-install as a library. Fixes #8419
| @@ -425,6 +425,8 @@ test-suite integration-tests2 | |||
| , process | |||
| , tasty >= 1.2.3 && <1.6 | |||
There was a problem hiding this comment.
sequentialTestGroup is only available since tasty-1.5
| , tasty >= 1.2.3 && <1.6 | |
| , tasty >= 1.5 && <1.6 |
| @@ -113,22 +118,42 @@ main = do | |||
| defaultMainWithIngredients | |||
| (defaultIngredients ++ [includingOptions projectConfigOptionDescriptions]) | |||
| ( withProjectConfig $ \config -> | |||
| testGroup | |||
| sequentialTestGroup | |||
There was a problem hiding this comment.
tasty story for sequential execution is admittedly finicky. If we want to run entire test suite in a single thread, I'd rather slap defaultMain $ localOption (NumThreads 1) $ ... on the very top level.
This fixes a problem where test output was leaking into GitHub Actions results (#8419), making the output unnecessarily verbose and harder to read.
This makes the test output much cleaner while still preserving all the diagnostic information when tests fail.
Fixes #8419
Please read Github PR Conventions and then fill in one of these two templates.
Template Α: This PR modifies behaviour or interface
Include the following checklist in your PR:
significance: significantin the changelog file.Template B: This PR does not modify behaviour or interface
E.g. the PR only touches documentation or tests, does refactorings, etc.
Include the following checklist in your PR: