-
-
Notifications
You must be signed in to change notification settings - Fork 650
Allow out-of-source testsuite builds. #1152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -75,7 +75,7 @@ else | |||
SHELL=/bin/bash | |||
endif | |||
QUIET=@ | |||
export RESULTS_DIR=test_results | |||
export RESULTS_DIR=./test_results |
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.
Make this: export RESULTS_DIR=.$(DSEP)test_results
And move it below the if wi32 block just below this line and I believe that'll make it work on win32.
Thanks for doing this cleanup. Get it working on win32 and I'll merge it in. |
Applied your suggested change – let's see how it fares on the Win32 box this time (unfortunately, I don't have a Win32 VM around for testing right now). Also submitted a DMD issue, http://d.puremagic.com/issues/show_bug.cgi?id=8736, because this isn't the first time I've hit this issue. |
Grf.. ok, I'll go poke at this one on a win32 box. I hate that part of the tester, it's twitchy as hell. But all in the service of not having to maintain two test drivers. |
Ah, the problem is of course that |
There's multiple problems, and unfortunately I don't have time to drive solutions, so you're going to have to. A (potentially incomplete) list:
I do like the fixes you've proposed here.. but they need to work on windows too. |
This is… interesting. GitHub shows commits here that are already in master. Yes, I checked the SHA-1 hashes, it's not a botched merge. |
@braddr: You might want to review this. Seems to work on Windows now. |
Fixed botched merge and adapted a test case that has been added in the meantime. |
Win32 fails right now, but that's an auto tester issue (can't connect to GitHub). |
Would you mind undoing the change from test_results to generated? I like fixing all the call sites and appreciate that changing it helped find them, but I don't want to break anyone who's scripted against the current output directory name. |
I agree, but can we have the autotester run it with a different output directory name? The reason I left the change in is so that people don't accidentally commit changes relying on test_result. |
Removed the commit. |
I agree that it'd probably be a good idea for the auto-tester to force a different name to help catch regressions. I'll add that to my todo list. It ought to be trivial. |
As trivial as specifying |
yup |
Allow out-of-source testsuite builds.
Thanks! |
Previously, "./" was prefixed to $RESULTS_DIR in several places, while $RESULTS_DIR == "test_result" was assumed in others.
This is useful for integrating the DMD test suite with the build system of other projects, e.g. LDC (via https://github.com/ldc-developers/dmd-testsuite).