snowleopard / hadrian

Hadrian: a new build system for the Glasgow Haskell Compiler. Now merged into the GHC tree!
https://gitlab.haskell.org/ghc/ghc/tree/master/hadrian
MIT License
208 stars 39 forks source link

Add options for running testsuite tests #605

Closed chitrak7 closed 5 years ago

chitrak7 commented 6 years ago

@snowleopard @izgzhen @angerman Can you please tell me how I should add support for more options for testing. More specifically, how should these arguments look like.

angerman commented 6 years ago

@chitrak7 I'm a bit lost on you question. Could you expand it a bit and maybe give a concrete example you have in mind?

snowleopard commented 6 years ago

@chitrak7 Why don't you make a proposal on how you think testsuite options should be implemented and we give you feedback? It's your GSoC project, so you should feel free to propose and make some design decisions yourself.

chitrak7 commented 6 years ago

Okay @snowleopard, Give me a day and I will propose a decent structure.

angerman commented 6 years ago

@chitrak7, any update on this? Do you need help with something specific?

chitrak7 commented 6 years ago

@angerman @snowleopard I believe that the required structure is already in place, all that we need is tweaking and implementing them. First of all, along with command line arguments, there are a few test suite arguments here. Their defination is near complete. Then, there is a file "Settings/Builder/RunTest". I believe this is supposed to generate configuration file for runtest.py. The implementation seemed to be correct, all we need there is a need rule and a few minor tweaks. After this, all we will need is write up the rules for the testsuite. Before I begin, can you tell me why the module "Settings/Builder/RunTest" is not being used at present? Is there something wrong with it or its just a TODO.

snowleopard commented 6 years ago

@chitrak7 The module Setting/Builder/RunTest was added by @sighingnow who was doing some work on the testsuite at some point -- he might be able to comment on his plans for this module.

sighingnow commented 6 years ago

Currently we already have hadrian test (not available on Windows). You can try with:

hadrian test --test-only="T13215"

can you tell me why the module "Settings/Builder/RunTest" is not being used at present?

The module "Settings/Builder/RunTest" mainly exports the runTestBuilderArgs and is used here: https://github.com/snowleopard/hadrian/blob/3837187e57bfcc9c00a717eb70cb3e9271525047/src/Settings/Default.hs#L141

all we will need is write up the rules for the testsuite

We already have https://github.com/snowleopard/hadrian/blob/3837187e57bfcc9c00a717eb70cb3e9271525047/src/Rules/Test.hs#L14

Now we already have several command-line flags for running testsuite, see PR #495 and https://github.com/snowleopard/hadrian/blob/3837187e57bfcc9c00a717eb70cb3e9271525047/src/Settings/Builders/RunTest.hs#L89-L104 . We already have:

@chitrak7 There are still many TODO in Settings/Builders/RunTest.hs which I'm not familiar with. These flags are not used frequently during developing ghc. I can suggest some good start point for this part of code:

chitrak7 commented 6 years ago

@sighingnow Thnks for the detailed reply. Can you also tell me why "runTestBuilderArgs" is not used for configuring runtest.py file?

@angerman @snowleopard I think we should ignore my previous pull and build validate rule again over @sighingnow's implementation. All setting related to testing should be limited to RunTest module only. Also, taking it out of Settings.Builder.Make module will help as we are unnecessarily configuring the testsuite arguments for every make command.

sighingnow commented 6 years ago

why "runTestBuilderArgs" is not used for configuring runtest.py file?

We do use runTestBuilderArgs to configure runtest.py. runTestBuilderArgs only work when the builder is RunTest. https://github.com/snowleopard/hadrian/blob/3837187e57bfcc9c00a717eb70cb3e9271525047/src/Settings/Builders/RunTest.hs#L13

In Rules/Test.hs, we do use buildWithCmdOptions with RunTest target. https://github.com/snowleopard/hadrian/blob/3837187e57bfcc9c00a717eb70cb3e9271525047/src/Rules/Test.hs#L64

chitrak7 commented 6 years ago

@sighingnow For a lot of test cases, an odd error is coming. For example with test cabal05, on executing the following command: sudo ./build.sh test -V --only="cabal05" The error that I encountr is: Wrong exit code for cabal05()(expected 0 , actual 2 ) Stderr ( cabal05 ): Makefile:2: /mk/boilerplate.mk: No such file or directory Makefile:3: /mk/test.mk: No such file or directory make: *** No rule to make target '/mk/test.mk'.

When I opened the folder contatining this test, I found that oddly, the value of TOP was not set in the Makefile, due to which, mk/boilerplate.mk and test.mk was not found. Currently, I am stuck on this problem. Can you help me how to approach this.

chitrak7 commented 6 years ago

@snowleopard I am thinking to move list of builder arguments for testsuite from Settings.Builder.Make to Settings.Builder.RunTest. I believe it will improve speed and accessibility as the arguments given to testsuite will become complex as we proceed in the project.

alpmestan commented 6 years ago

When I opened the folder contatining this test, I found that oddly, the value of TOP was not set in the Makefile, due to which, mk/boilerplate.mk and test.mk was not found. Currently, I am stuck on this problem. Can you help me how to approach this.

Do you run hadrian from the root of the ghc tree?

sighingnow commented 6 years ago

I found that oddly, the value of TOP was not set in the Makefile, due to which, mk/boilerplate.mk and test.mk was not found.

@chitrak7 You could try to add the TOP environment variable here:

https://github.com/snowleopard/hadrian/blob/c0292ffcaee940cfd3a81a878a680e0e273c10bc/src/Rules/Test.hs#L58-L61

chitrak7 commented 6 years ago

@alpmestan I am running the command from hadrian directory. The exact command is: sudo ./build.sh test -V --only="cabal05"

@sighingnow The TOP variable is actually the relative path of tests Makefile to testsuite folder. For example "../../.." or "../../../.." . I don't think I will be able to set this directly as an environment.

sighingnow commented 6 years ago

@chitrak7 That doesn't matter. All $(TOP) in ghc's tests are used to find mk/test.mk and mk/mk/boilerplate.mk:

include $(TOP)/mk/boilerplate.mk
include $(TOP)/mk/test.mk

Setting TOP as root -/- "testsuite" should be enough to address this problem.

chitrak7 commented 6 years ago

Yeah, It just came to my mind right now!! But still, a question remains, why is Hadrian not able to do so by its own. I believe there must be some internal configuration process which the test suite goes through before tests are run.

I ran the test using make with the command: sudo make TEST="cabal05" CLEANUP=0 and then checked the makefile, and the TOP setting was configued. I am afraid that there might be other settings that hadrian is not configuring that make configures on its own, causing other problems.

sighingnow commented 6 years ago

causing other problems.

Could you provide me some cases of this kind of failure? I will give it a try later.

chitrak7 commented 6 years ago

@sighingnow actually top environment was set by runtest.py file only. We can do it by adding an additional argument like this: arg "-e", arg $ "config.top=" ++ show (top -/- "testsuite")

chitrak7 commented 6 years ago

@snowleopard @angerman I wanted to know the need of a separate validate rule. After support of test is completed, we can validate by simply running test fast. So do we really need a separate validate rule?

snowleopard commented 6 years ago

@chitrak7 It would be best to mirror all functionality of the Make build system in Hadrian, since it would make it more convenient for GHC developers to switch to Hadrian.

Here is a task for you: learn and document the differences between the validate script and the testsuite -- that would be useful both for you and for others. You could add this to hadrian/doc/testsuite.md and link to it from the Hadrian's README. Then, if you still think that the validate script is redundant, you should create a new issue and invite GHC developers to discuss its removal.

chitrak7 commented 6 years ago

@angerman @snowleopard @sighingnow Multiple test cases which are supposed to fail are passing unexpectedly. For all these test cases a warning is being generated.
*** framework warning for T4474a(numfield-no-expected) No expected value found for bytes allocated in num_field check.

sighingnow commented 6 years ago

To run perf test needs setting WORDSIZE properly. See:

https://github.com/ghc/ghc/blob/a5446c4501cf30aa06eab636823d031bb98db739/testsuite/mk/test.mk#L239

and

https://github.com/ghc/ghc/blob/a5446c4501cf30aa06eab636823d031bb98db739/testsuite/mk/ghc-config.hs#L11

The ghc-config.hs is built and run by testsuite/mk/boilerplate.mk.

I think current it's ok to just set WORDSIZE as 64 in RunTest.hs. Then we could focus on other major tasks and fix this setting later.

chitrak7 commented 6 years ago

Thanks @sighingnow the errors have reduced now. But still many unexpected passes are coming. I am looking further into it.

snowleopard commented 5 years ago

I believe this has been fixed.