Fix various issues with testsuite code on Windows
Concern Raiseda3704409acc3

Authored by Phyx on Dec 28 2016, 4:04 AM.

Description

Fix various issues with testsuite code on Windows

Summary:
Previously we would make direct calls to diff using os.system.
On Windows os.system is implemented using the standard
idiom CreateProcess .. WaitForSingleObject ...

This again runs afoul with the _exec behaviour on Windows. So we ran
into some trouble where sometimes diff would return before it's done.

On tests which run multiple ways, such as 8086 what happens is that
we think the diff is done and continue. The next way tries to set things
up again by removing any previous directory. This would then fail with
and error saying the directory can't be removed. Which is true, because
the previous diff code/child is still running.

We shouldn't make any external calls to anything using os.system.
Instead just use runCmd which uses timeout. This also ensures that if
we hit the cygwin bug where diff or any other utility hangs, we kill it and
continue and not hang the entire test and leave hanging processes.

Further more we also:
Ignore error lines from removeFile from tools in the testsuite. This is a rather large
hammer to work around the fact that hsc2hs often tries to remove it's own file too early.
When this is patched the workaround can be removed. See Trac Trac #9775

We mark prog003 as skip. Since this test randomly fails and passes. For stability it's disabled
but it is a genuine bug which we should find. It's something with interface files being
overwritten. See Trac Trac #11317

when rmtree hits a readonly file, the onerror handler is raised afterwards but not
during the tree walk. It doesn't allow you to recover and continue as we thought.
Instead you have to explicitly start again. This is why sometimes even though we
call cleanup before os.mkdirs, it would sometimes fail with an error that the
folder already exists. So we now do a second walk.

A new verbosity level (4) will strip the silent flags from MAKE invocations so you can actually
see what's going on.

Test Plan: ./validate on build bots.

Reviewers: bgamari, austin

Reviewed By: bgamari

Subscribers: mpickering, thomie, #ghc_windows_task_force

Differential Revision: https://phabricator.haskell.org/D2894

GHC Trac Issues: Trac #12661, Trac #11317, Trac #9775

nomeata raised a concern with this commit.Feb 15 2017, 10:40 PM
nomeata added a subscriber: nomeata.

Sorry to jump on this after it has been commited, but I have doubts about the VERBOSE setting. If increasing the verbosity can make tests fail, then that is a harsh violation of the principle of least surprise!

Better introduce a new settings called DEBUG_MAKE or similar, with clear documentation that this might produce spurious failures.

(Or make sure that no extra tests fail with this settings.)

I guess I will take this discussion to D3141.

This commit now has outstanding concerns.Feb 15 2017, 10:40 PM