-
Notifications
You must be signed in to change notification settings - Fork 37
Added support for testsuite #602
Changes from all commits
39e3cc2
0d6050b
fffd876
a6b5e05
cf67794
b98524f
d06ef68
5961be4
29a6a0d
d886a74
cf9a443
71d87c8
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 |
---|---|---|
|
@@ -5,6 +5,7 @@ import Expression | |
import GHC | ||
import Oracles.Flag | ||
import Oracles.Setting | ||
import Settings | ||
import Target | ||
import Utilities | ||
|
||
|
@@ -63,13 +64,23 @@ testRules = do | |
-- Execute the test target. | ||
buildWithCmdOptions env $ target (vanillaContext Stage2 compiler) RunTest [] [] | ||
|
||
-- | Build extra programs required by testsuite | ||
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. The code below suggests that there are not only testsuite programs, but also testsuite libraries. 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. @chitrak7 So, do you really need to handle libraries here? 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. I have to handle rts library. I just cannot yet figure out how. 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. Why do you need to handle 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. I still don't understand why 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. Some files are missing in rts library causing multiple failures. At the moment, I cannot figure out which files are missing. Or they may be missing includes. I have to look into this matter further. 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. I don't understand how The comment for this function says "Build extra programs required by testsuite", but the implementation can also build libraries. This is an inconsistency that should be fixed. 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. Okay! I think I should create an issue regarding this and temporarily install required programs only. 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. Resolved |
||
needTestsuiteBuilders :: Action () | ||
needTestsuiteBuilders = do | ||
targets <- mapM (needfile Stage1) =<< testsuitePackages | ||
need targets | ||
where | ||
needfile :: Stage -> Package -> Action FilePath | ||
needfile stage pkg = programPath =<< programContext stage pkg | ||
|
||
|
||
needTestBuilders :: Action () | ||
needTestBuilders = do | ||
needBuilder $ Ghc CompileHs Stage2 | ||
needBuilder $ GhcPkg Update Stage1 | ||
needBuilder Hp2Ps | ||
needBuilder Hpc | ||
needBuilder (Hsc2Hs Stage1) | ||
needTestsuiteBuilders | ||
|
||
-- | Extra flags to send to the Haskell compiler to run tests. | ||
runTestGhcFlags :: Action String | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,7 @@ | ||
module Settings.Builders.Make (makeBuilderArgs) where | ||
module Settings.Builders.Make (makeBuilderArgs, validateBuilderArgs) where | ||
|
||
import GHC | ||
import Oracles.Setting | ||
import Rules.Gmp | ||
import Rules.Libffi | ||
import Settings.Builders.Common | ||
|
@@ -13,5 +15,22 @@ makeBuilderArgs = do | |
mconcat | ||
[ builder (Make gmpPath ) ? pure ["MAKEFLAGS=-j" ++ t] | ||
, builder (Make libffiPath ) ? pure ["MAKEFLAGS=-j" ++ t, "install"] | ||
, builder (Make "testsuite/tests") ? pure ["THREADS=" ++ t, "fast"] | ||
] | ||
|
||
validateBuilderArgs :: Args | ||
validateBuilderArgs = builder (Make "testsuite/tests") ? do | ||
threads <- shakeThreads <$> expr getShakeOptions | ||
top <- expr topDirectory | ||
compiler <- expr $ fullpath ghc | ||
checkPpr <- expr $ fullpath checkPpr | ||
checkApiAnnotations <- expr $ fullpath checkApiAnnotations | ||
return [ "fast" | ||
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. Is the 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. No, this is the default validate argument. Speed can be specified by running ./build.sh test and not validate. 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. Worst case, we can later arrange to support 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. Yeah. I can do that right now. 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. OK, I guess we can leave this for later. |
||
, "THREADS=" ++ show threads | ||
, "TEST_HC=" ++ (top -/- compiler) | ||
, "CHECK_PPR=" ++ (top -/- checkPpr) | ||
, "CHECK_API_ANNOTATIONS=" ++ (top -/- checkApiAnnotations) | ||
] | ||
where | ||
fullpath :: Package -> Action FilePath | ||
fullpath pkg = programPath =<< programContext Stage1 pkg | ||
|
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.
Shake already provides a
--verbose
option, let's reuse it.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.
The verbose option of Shake is boolean, whereas test suite supports 5 levels of verbosity. This is why I chose to pass a separate verbose argument.
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.
I think that it may be convenient to control these two verbosity settings separately: you might want verbose testing, but not Shake diagnostic info. And vice versa. So, let's keep this flag.