Skip to content
This repository has been archived by the owner on Aug 2, 2020. It is now read-only.

Add validation support #187

Closed
1 of 8 tasks
snowleopard opened this issue Jan 22, 2016 · 29 comments
Closed
1 of 8 tasks

Add validation support #187

snowleopard opened this issue Jan 22, 2016 · 29 comments

Comments

@snowleopard
Copy link
Owner

We need to add support for running automated GHC tests when the build system is run with test target.

Below are specific subtasks we identified in this thread (see discussions below):

  • Implement a temporary target validate which simply runs make fast in testsuite/tests.
  • Support a flag like --test-hc=... to allow specifying a Haskell compiler to use for running tests. Possible options could be stage0 (bootstrapping compiler), stage1, stage2 (default), stage3 and path/to/compiler to specify any other compiler. We need to support paths with spaces and sometimes expand paths using environment variable PATH.
  • Detect correct paths to ghc-pkg, hpc, runghc, haddock, hsc2hs, hp2ps in accordance with the chosen --test-hc. See testsuite/mk/boilerplate.mk.
  • Don't trust settings like getLibraryWays in the build system during testing. Instead probe the file system to see which ways are actually there. See HAVE_PROFILING in testsuite/mk/test.mk. Other options can be found via ghc --info and ghc +RTS --info. See testsuite/mk/ghc-config.hs, which generates a Makefile with options passed to the Python driver from testsuite/mk/test.mk.
  • Use getPackages to generate all --rootdir flags for passing to the python runtests.py script.
  • Run only one specific test: build.sh test --only=....
  • Run local tests only when in a specific source folder: build.sh test --local-only. If possible detect this automatically, so --local-only flag is not required.
  • Support --way=... flag. Example: make TEST='tc002 tc002' WAY='normal optasm'
@snowleopard
Copy link
Owner Author

Several quick attempts to reuse the validate script have failed, so I decided to start implementing a proper test rule in the new build system. (Note, I still rely on the old Python scripts for testing -- rewriting them seems to be a major undertaking.) The test rule does work for me on Windows, but the functionality is very limited at the moment. You can give it a try by running:

shake-build/build.sh test

This should run (some) tests. I added a section on testing to the README: https://github.com/snowleopard/shaking-up-ghc#testing.

@thomie
Copy link

thomie commented Jan 28, 2016

The GHC testsuite is make-based, but it is completely separate from the GHC build system. So instead of reimplementing the logic from testsuite/mk and testsuite/tests/Makefile, by calling python testsuite/driver/runtests.py with lots of arguments, you can just run one of the following commands, and everything should just work (copy-pasted from toplevel Makefile):

.PHONY: fasttest
fasttest:
    $(MAKE) -C testsuite/tests CLEANUP=1 SUMMARY_FILE=../../testsuite_summary.txt fast

.PHONY: test
test:
    $(MAKE) -C testsuite/tests CLEANUP=1 SUMMARY_FILE=../../testsuite_summary.txt

.PHONY: slowtest fulltest
slowtest fulltest:
    $(MAKE) -C testsuite/tests CLEANUP=1 SUMMARY_FILE=../../testsuite_summary.txt slow

That said, you can be even more lazy, and just tell GHC developers that the testsuite can only be run via validate, or from inside the testsuite directory. Those toplevel Makefile targets never did add much value.

I don't know which problems you ran into with validate. Its main purpose is to do a clean build with mk/flavours/validate.mk settings, create a binary distribution, install it somewhere, and run the testsuite using that installed version.

This is the main build command:

$make -j$threads

The build system knows to use mk/flavours/validate.mk through some magic in mk/custom-settings.mk.

If you have any questions about the current build system or testsuite, please ask.

@snowleopard
Copy link
Owner Author

@thomie Many thanks for the details. I think I did manage to understand this overall structure while trying to reuse validate, but it's very helpful to have it all explained in one place.

The key obstacle for simply calling validate is that we are in the process of separating build results from GHC sources (see #113), so some build artefacts are not where validate & make expect them to be, hence initiating rebuilds. Anyway, validate script doesn't look too complicated and I hope I'll be able to fully consume it into the new build system soon.

@thomie
Copy link

thomie commented Jan 28, 2016

The key obstacle for simply calling validate is that we are in the process of separating build results from GHC sources

Maybe just leave things where they are now? Then those GHC bugs you ran into in #131 and here can be fixed at some later time.

Even when not relying directly on any of the artifacts from the old build system, it might allow you to run both systems separately, logging all shell commands and diffing the logfiles to spot inconsistencies.

make clean in a libraries or utils directory being the same as deleting a dist or dist-install subdirectory can also be convenient.

Anyway, my main point above was: call make -C testsuite/tests to run the testsuite. Here's another reason: running make test / make allow / make fast / make slow in one of testsuite/tests subdirectories should keep working, or have an equivalent replacement. If you duplicate some of the settings from the make based testsuite driver, but not all, we'll have to keep them in sync manually. So I think the explicit call to python in selftest should be deleted, or all (or at least all Makefile based parts) of the testsuite should be rewritten to Haskell (something @ndmitchell once mentioned on twitter would be worth doing).

@snowleopard
Copy link
Owner Author

Maybe just leave things where they are now?

But this requires extra work. The stageN naming convention is cleaner and more internally consistent, which simplifies the new build system. Adding support for exact compatibility with dist* naming convention is certainly possible, but it doesn't feel a very productive way to spend my limited time right now. I'd rather spend it on getting rid of all makefiles, which is the main goal of this project.

Here's another reason: running make test / make allow / make fast / make slow in one of testsuite/tests subdirectories should keep working, or have an equivalent replacement.

Yes, sure. We will provide equivalent replacements.

So I think the explicit call to python in selftest should be deleted, or all (or at least all Makefile based parts) of the testsuite should be rewritten to Haskell.

You probably meant test. I am not sure I understand: I will call the python script in exactly the same way as it is currently called from make. And yes, we'll try to get rid of as many makefiles as possible. This is what the project is about. Of course we can still run a make script from within the new build system when we don't have a good control over its contents, e.g. the make script in in-tree GMP library. But all other makefiles are scheduled for termination :)

@thomie
Copy link

thomie commented Jan 28, 2016

But this requires extra work.

Ok, sorry, I wrote my comment thinking it would be less work. A better naming scheme than dist* is also nice.

all other makefiles are scheduled for termination

I won't stop you, but as I said before, the make based parts of the testsuite are completely separate from the main build system. They are also fairly trivial and don't cause anyone problems at the moment, so you could easily defer this whole sub-project to a later time, especially if you have limited time now.

The Makefiles in the testsuite currently serve two different purposes:

  • Make test / allow / fast / slow etc. targets available, that call the python driver
  • Contain implementations for most run_command tests, that are called by the python driver.

Do you plan to move the latter to Haskell as well?

@thomie
Copy link

thomie commented Jan 28, 2016

the make based parts of the testsuite are completely separate from the main build system

To clarify: you could delete all .mk and Makefiles not in the testsuite directory, and the testsuite still works.

This also serves a purpose: you can run the testsuite with an external compiler (with the TEST_HC env variable). To prevent GHC developers accidentally relying on build system settings for one of the tests, which won't be accurate when using the external compiler, it is useful to keep the build system and the testsuite separate. At least that's how it is designed now, but I suppose there's ways to guarantee this separation in Shake as well. One recent example is testing if libraries are built a certain way or not, or contain debug symbols or not. The testsuite should not rely on build system settings for this, but instead ask ghc-pkg or probe the filesystem or have some other way to figure this out.

I have another question about what the story will look like for end users. Currently, they can get a binary distribution and run ./configure && make test && make install. How will that work without Makefiles?

I did a little reading of the shaking-up-ghc source. So much nicer than the old system! Awesome job.

@snowleopard
Copy link
Owner Author

To clarify: you could delete all .mk and Makefiles not in the testsuite directory, and the testsuite still works.

Oh, I feel very dumb! I think I never tried running make fast from the teststuite directory directly (I only tried the validate script which somehow forced rebuilds). Let me check this. If it works then we'll have a great temporary solution.

The Makefiles in the testsuite currently serve two different purposes:

  • Make test / allow / fast / slow etc. targets available, that call the python driver
  • Contain implementations for most run_command tests, that are called by the python driver.
    Do you plan to move the latter to Haskell as well?

Let me think about this for some time. I don't know this part of the testsuite well yet.

I have another question about what the story will look like for end users. Currently, they can get a binary distribution and run ./configure && make test && make install. How will that work without Makefiles?

I think this will work as follows:

./configure
shake-build/build.sh test
shake-build/build.sh install

I did a little reading of the shaking-up-ghc source. So much nicer than the old system! Awesome job.

Happy to hear you like it! :-) There are a couple of further refactorings which I hope will make the code even better.

@thomie
Copy link

thomie commented Jan 28, 2016

some build artefacts are not where validate & make expect them to be, hence initiating rebuilds

I only tried the validate script which somehow forced rebuilds

Aha! I think I understand your problem.

validate does this on purpose though, to make sure there is nothing left behind from an earlier build:

$make maintainer-clean NO_CLEAN_GMP=YES

You can call ./validate --no-clean to avoid it.

The other reason it will do rebuilds, is that your development build will probably have used different settings than those in validate.mk. That's why you should avoid calling validate in the git tree where you are currently working, but use something like lndir or a separate clone / working directory or let Travis/Harbormaster do it.

@snowleopard
Copy link
Owner Author

@thomie make fast works from the testsuite/tests folder and so does make -C testsuite/tests fast from the root. I only need to build inplace/bin/hpc and I should be able to use it without the make build system. Hurray! Hopefully I'll fix this today.

Note: ./validate --no-clean --fast still initiates a complete rebuild of GHC. I don't know why.

@thomie
Copy link

thomie commented Jan 28, 2016

Note: ./validate --no-clean --fast still initiates a complete rebuild of GHC. I don't know why.

How did you make your first build? If with the new build system, then yes, it will do a complete rebuild, because validate calls make to do a build, and the new build system puts files in different places as I understand it. But you know this, so I'm confused.

Maybe you're looking for ./validate --testsuite-only?

snowleopard added a commit that referenced this issue Jan 29, 2016
snowleopard added a commit that referenced this issue Jan 29, 2016
@snowleopard
Copy link
Owner Author

How did you make your first build?

I tried the following: run the make build system first, then run the new build system on top of it, then run ./validate --no-clean --fast (with the intention to later iterate the last two). This forced rebuilds for me.

Anyway, I've added some bits to the new build system and now we can simply say:

shake-build/build.sh validate

and it will run make fast in testsuite/tests. Seems to work for me. I'll give it a couple of tests tomorrow and will send an update to ghc-devs.

This is intended as a temporary solution. I'll keep working on the test target, to call python directly.

@thomie
Copy link

thomie commented Jan 29, 2016

I tried the following: run the make build system first, then run the new build system on top of it, then run ./validate --no-clean --fast (with the intention to later iterate the last two). This forced rebuilds for me.

Why do you think it should not do a rebuild?

@snowleopard
Copy link
Owner Author

Why do you think it should not do a rebuild?

  • The new build system builds inplace/bin/ghc-stage2 with a later time stamp, which could make make happy. But I guess ghc-pkg's later time stamp invalidated all package-data.mk files. This wouldn't be fatal for the new build system, since their contents wouldn't change. But make doesn't like this and rebuilds everything.
  • There was also hope that validate is not directly linked to the make build system at all. At least this wasn't obvious from my quick examination of makefiles.

@thomie
Copy link

thomie commented Jan 29, 2016

The new build system builds inplace/bin/ghc-stage2 with a later time stamp, which could make make happy.

Maybe I'm misinterpreting this comment, but make (make all) does not only depend on the timestamps of the programs in inplace/bin. The all target also depends on all the libraries. So if you touch compiler/main/GHC.hs for example, and then update the timestamp of inplace/bin/ghc-stage2 (via the new build system), running make is not a no-op.

run the new build system on top of it, then run ./validate --no-clean --fast (with the intention to later iterate the last two)

So if the "iterating" part here also includes making changes to source files (which I guess it does, why would you otherwise be iterating), this can never work without duplicating work.

ghc-pkg

Updating the timestamp of inplace/bin/ghc-pkg will indeed trigger a terrible chain of rebuilds, without LAX_DEPENDENCIES=YES (which mk/flavours/validate.mk doesn't set).

hope that validate is not directly linked to the make build system

I don't understand what you mean by this.

validate --no-clean --fast does this basically:

make
make fasttest

What had you hoped it would do? How would you make it not linked to the make build system, and still do something useful?

@snowleopard
Copy link
Owner Author

validate --no-clean --fast does this basically:

make
make fasttest

What had you hoped it would do?

Well, if it was written this way then surely I wouldn't have any hopes! :) Yet I found this:

$make $MAKE_TEST_TARGET stage=2 $BINDIST $TEST_VERBOSITY THREADS=$threads 2>&1 | tee testlog

Plus quite a lot of if statements above. I guess this is the line I didn't spot:

$make -j$threads

I don't think this deserves any further investigation. Clearly it was just my oversight.

snowleopard added a commit that referenced this issue Jan 30, 2016
snowleopard added a commit that referenced this issue Jan 31, 2016
@snowleopard
Copy link
Owner Author

@thomie I'd like to come back to one of the points you made earlier:

This also serves a purpose: you can run the testsuite with an external compiler (with the TEST_HC env variable). To prevent GHC developers accidentally relying on build system settings for one of the tests, which won't be accurate when using the external compiler, it is useful to keep the build system and the testsuite separate. At least that's how it is designed now, but I suppose there's ways to guarantee this separation in Shake as well. One recent example is testing if libraries are built a certain way or not, or contain debug symbols or not. The testsuite should not rely on build system settings for this, but instead ask ghc-pkg or probe the filesystem or have some other way to figure this out.

Let me try to condense this into a specification on how a future test target should work:

  • Support a flag like --test-hc=... to allow specifying a Haskell compiler to use for running tests. Possible options could be stage0 (bootstrapping compiler), stage1, stage2 (default), stage3 and path/to/compiler to specify any other compiler.
  • What about ghc-pkg and hpc? Do we always use the ones built by the build system?
  • Don't trust settings like getLibraryWays in the build system during testing. Instead probe the file system to see which ways are actually there.
  • On a similar note, there is getPackages, which we currently use to find out the libraries enabled in the build system. Can we rely on it, or shall we scan the file system and attempt to detect which ones should be tested instead? My intuition is that it is OK to rely on getPackages, but maybe I am wrong.

Plus a couple of other requirements collected elsewhere:

  • Run only one specific test: build.sh test --only=....
  • When local tests when in a specific source folder: build.sh test --local-only.

If the above sounds right I will put it on top and create subtasks for easier progress monitoring.

@thomie
Copy link

thomie commented Jan 31, 2016

The simple answer is, reimplement these 3 files:

$ ls testsuite/mk/
boilerplate.mk  ghc-config.hs  test.mk
  • Support a flag like --test-hc=... to allow specifying a Haskell
    compiler to use for running tests. Possible options could be stage0
    (bootstrapping compiler), stage1, stage2 (default), stage3 and
    path/to/compiler to specify any other compiler.
    Yes. Currently also supported: both "ghc" and "/usr/bin/ghc", both
    "C:/path/to/ghc.exe" and "/c/path/to/ghc.exe", and paths that contain
    spaces.
  • What about ghc-pkg and hpc? Do we always use the ones built by the
    build system?
    No. Always use the one that belongs to TEST_HC. The tedious logic and
    Windows compatibility is in testsuite/mk/boilerplate.mk. Other tools to
    take care of:
    • runghc
    • haddock (if it isn't available, pass --config.haddock=, so some tests
      can be skipped)
    • hsc2hs
    • hp2ps

Having separate flags to override these isn't necessary I think, but the
Python driver needs to know where they are. I suppose this logic could be
pushed into the Python driver as well.

Don't trust settings like getLibraryWays in the build system during
testing. Instead probe the file system to see which ways are actually there.

Yes. See for example HAVE_PROFILING in testsuite/mk/test.mk.
Other options can be gotten from running ghc --info and ghc +RTS --info. See testsuite/mk/ghc-config.hs, which generates a Makefile with
options, which are then passed to the Python driver from testsuite/mk/ test.mk.

  • On a similar note, there is getPackages, which we currently use to
    find out the libraries enabled in the build system. Can we rely on it, or
    shall we scan the file system and attempt to detect which ones should be
    tested instead? My intuition is that it is OK to rely on getPackages,
    but maybe I am wrong.
    The Python driver calls ghc-pkg to find out which packages are installed,
    so I don't think you have to do anything here. See the function _reqlib
    in testsuite/driver/testlib.py for details.
  • Run only one specific test: build.sh test --only=....
    Yes, and also a --way is necessary. (a common invocation of the
    testsuite: make TEST='tc002 tc002' WAY='normal optasm')

Some more variables are documented here:
https://ghc.haskell.org/trac/ghc/wiki/Building/RunningTests/Settings
Or look at what is accepted by 'testsuite/driver/runtests.py`.

  • When local tests when in a specific source folder: build.sh test
    --local-only.
    Yes, though this happens automagically with the make based system.

@snowleopard
Copy link
Owner Author

The Python driver calls ghc-pkg to find out which packages are installed,
so I don't think you have to do anything here.

Hmm, but what about this code in testsuite/tests/Makefile:

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

# The libraries that we actually know about. We don't want to test
# extralibs that are in our tree but which we haven't built, and
# we don't want to test unix on Windows or Win32 on non-Windows.
LIBRARIES := $(shell '$(GHC_PKG)' list --simple-output --names-only)

ifeq "$(findstring base,$(LIBRARIES))" ""
$(error base library does not seem to be installed)
endif

# Now find the "tests" directories of those libraries, where they exist
LIBRARY_TEST_PATHS := $(wildcard $(patsubst %, $(TOP)/../libraries/%/tests, $(LIBRARIES))) \
      $(wildcard $(patsubst %, $(TOP)/../libraries/%/tests-ghc, $(LIBRARIES)))

# Add tests from packages
RUNTEST_OPTS += $(patsubst %, --rootdir=%, $(LIBRARY_TEST_PATHS))

At the moment I use getPackages to generate all --rootdir flags for passing to the python script. And this seems to be the right thing to do.

Yes, and also a --way is necessary.

Indeed. Will add it as a subtask too.

Yes, though this happens automagically with the make based system.

I see. It may be possible to keep it automagical in the new build system too.

@thomie
Copy link

thomie commented Jan 31, 2016

At the moment I use getPackages to generate all --rootdir flags for passing to the python script.

Ok, that seems fine. These --rootdir flags are used for finding out where the tests are (or more specifically, which directories should be scanned for *.T files).

@snowleopard
Copy link
Owner Author

Thanks @thomie! I've added subtasks to the top of the issue.

I will switch to more urgent issues now, tagged with tree-tremble. Maybe somebody else will pick this up in the meanwhile.

@izgzhen izgzhen added this to Testing & Infra in GHC Merge Jul 18, 2017
@chitrak7
Copy link
Contributor

@alpmestan @snowleopard Hi,
I was looking at #531 and I believe that currently, we are giving fixed arguments to make command. WREF: Settings/Builders/RunTest.hs. I think that the above task could be achieved by looking cmdOptions for possible user command options an then implementing it, otherwise using the default arguments. Am I going in the right track to tackle this issue?

@chitrak7
Copy link
Contributor

@snowleopard @izgzhen
Currently, a very elementary validate feature is added to Hadrian. A lot of features are supported by testsuite but we are not currently using any of them. Instead, we are using a set of default hardcoded flags. In the summers I plan to:
Implement support of command line flags like --VERBOSE, “fasttest” ,“slowtest” etc. [Expected 5 days to completely understand the code]
Add support to test single file/single directory. This feature is available in make already, all we need to do is provide command line arguments. [Expected : 3 days]
Add support of specifying the compiler to be used, and the directories where the compiler and its releveand executables are stored. [Expected 3 days]
Generate all --rootdir flags for runTests.py script [Expected 3 days]
[Total 14 days]
Is this approach sufficient to solve this issue. Also, is the time alloted by me for these issues practical.

@snowleopard
Copy link
Owner Author

@chitrak7 Sounds good to me, but this should better be discussed with GHC developers. You'll need to learn most common GHC development workflows and design validation features accordingly.

@chitrak7
Copy link
Contributor

chitrak7 commented Jun 15, 2018

@snowleopard #621 #602 have completed all but two checks here. Only --local-only and library ways remain now.

@chitrak7
Copy link
Contributor

Don't trust settings like getLibraryWays in the build system during testing. Instead probe the file system to see which ways are actually there. See HAVE_PROFILING in testsuite/mk/test.mk. Other options can be found via ghc --info and ghc +RTS --info. See testsuite/mk/ghc-config.hs, which generates a Makefile with options passed to the Python driver from testsuite/mk/test.mk.

@snowleopard Also, I wish to know what you mean when you say don't trust library ways? We are not generating library ways anywhere.

@chitrak7
Copy link
Contributor

Run local tests only when in a specific source folder: build.sh test --local-only. If possible detect this automatically, so --local-only flag is not required.
@snowleopard Also can you elaborate this issue.

@snowleopard
Copy link
Owner Author

@chitrak7 The idea here is that if you are working on a particular GHC component, say the base library, you can run build test from the base folder, perhaps with some additional flag, and it will run tests only for the base library. This is a lower priority feature.

@snowleopard
Copy link
Owner Author

This issue has been superseded by #669.

ghc-mirror-bot pushed a commit to ghc/ghc that referenced this issue Jul 29, 2019
snowleopard/hadrian#187 was superseded by
snowleopard/hadrian#669, which has also
been closed.

So, optimistically, dropping this as a limitation issue.
ghc-mirror-bot pushed a commit to ghc/ghc that referenced this issue Jul 29, 2019
snowleopard/hadrian#187 was superseded by
snowleopard/hadrian#669, which has also
been closed.

So, optimistically, dropping this as a limitation issue.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
GHC Merge
Testing & Infra
Development

No branches or pull requests

4 participants