Add and use RESULTS_TEST_DIR + OUTPUT_BASE + EXTRA_FILES in the bash scripts of DMD's testsuite by wilzbach · Pull Request #8038 · dlang/dmd

wilzbach · 2018-03-15T14:22:57Z

Turns out we can save quite a lot when we move the redundant definitions in the bash files to the new wrapper.
This also opens the opportunity to run the shell script in a temporary directory [1] or do other things as now most scripts don't have any hard-coded information anymore.

I'm not fixed about the names EXTRA_FILES could also be named SRC for example.

[1] this would mean e.g. that no manual cleaning of the generated files is necessary anymore, but it's just one potential opportunity. The big motivation of this PR is to remove the redundant declarations and thus let the test scripts focus on what's important: testing and not startup/cleanup code.

CC @marler8997 @timotheecour @CyberShadow

dlang-bot · 2018-03-15T14:22:58Z

Thanks for your pull request, @wilzbach!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + dmd#8038"

wilzbach · 2018-03-15T14:25:24Z

There's a bit of an overload with #8025, but the two PRs are independent and pursue different things (here common variables, in #8025 verbose stack trace)

wilzbach · 2018-03-15T14:26:07Z

test/compilable/jsonNoOutFile.sh

-$DMD -c -od=${dir} -Xi=compilerInfo compilable/extra-files/emptymain.d > ${output_file}
-rm -f emptymain.json || true
+$DMD -c -od=${RESULTS_TEST_DIR} -Xi=compilerInfo ${EXTRA_FILES}/emptymain.d
+rm -f emptymain.json


See also: #8025 (comment)

wilzbach · 2018-03-15T14:27:34Z

test/compilable/test6461.sh

+$DMD -m${MODEL} -od${RESULTS_OUTPUT} -I${src} ${src}/main.d ${RESULTS_OUTPUT}/a${LIBEXT} ${RESULTS_OUTPUT}/b${LIBEXT} || exit 1

-rm -f ${dir}/{a${LIBEXT} b${LIBEXT} main${EXE} main${OBJ}}
+rm -f ${RESULTS_OUTPUT}/{a${LIBEXT} b${LIBEXT} main${EXE} main${OBJ}}


That's one of the cases where just nuking the RESULTS_OUTPUT folder would make sense (and is probably a good default (if no error happened), but I went with keeping the existing behavior to make reviewing easy.

wilzbach · 2018-03-15T14:28:52Z

test/runnable/depsprot.sh

-echo >> ${output_file}
+echo "Dependencies file:"
+cat ${deps_file}
+echo


stdout is redirected to the output by default. See #8025 which makes this even more interesting with a verbose stack trace.

wilzbach · 2018-03-15T14:29:54Z

test/compilable/issue17167.sh

-rm -rf "${dir:?}"/"$TEST_NAME"
+rm -rf "${RESULTS_OUTPUT}"

 echo Success


This is superfluous too, but already removed in #8025

wilzbach · 2018-03-15T14:30:52Z

test/runnable/gdb15729.sh

-$DMD -g -m${MODEL} -I${src} -of${libname} -lib ${src}${SEP}lib15729.d || exit 1
-$DMD -g -m${MODEL} -I${src} -of${dir}${SEP}gdb15729${EXE} ${src}${SEP}gdb15729.d ${libname} || exit 1
+$DMD -g -m${MODEL} -I${EXTRA_FILES} -of${RESULTS_OUTPUT}${LIBEXT} -lib ${EXTRA_FILES}${SEP}lib15729.d || exit 1
+$DMD -g -m${MODEL} -I${EXTRA_FILES} -of${RESULTS_OUTPUT}${EXE} ${EXTRA_FILES}${SEP}gdb15729.d ${RESULTS_OUTPUT}${LIBEXT} || exit 1


All these || exit 1 should be removed to and be replaced with set -e, but that's a task for another PR.

marler8997 · 2018-03-15T16:25:44Z

test/sh_do_test.sh

 export TEST_DIR=$1 # TEST_DIR should be one of compilable, fail_compilation or runnable
 export TEST_NAME=$2 # name of the test, e.g. test12345
+export RESULTS_TEST_DIR=${RESULTS_DIR}/${TEST_DIR} # reference to the resulting test_dir folder, e.g .test_results/runnable
+export RESULTS_OUTPUT=${RESULTS_TEST_DIR}/${TEST_NAME} # reference to the resulting files without a suffix, e.g. test_results/runnabel/test123


Maybe ${RESULTS_OUTPUT_BASE} ? But it is used quite a bit...maybe it's OK as it is... Or maybe we don't need RESULTS, maybe OUTPUT_PREFIX? or OUTPUT_BASE?

Good idea - I went with OUTPUT_BASE, but I don't have any strong preference.

marler8997 · 2018-03-15T21:13:05Z

(For another PR)
Might be nice to add a README.md to this directory. That explains what variables are available to BASH tests. Maybe move the documentation from the Makefile to the README file and have it explain everything there.

wilzbach · 2018-03-22T21:48:48Z

(rebased as #8025 with the verbose xtracing is now in)

ibuclaw · 2018-03-25T08:19:04Z

This also opens the opportunity to run the shell script in a temporary directory

N.B. Many D tests are missing EXTRA_FILES (or maybe EXTRA_SOURCES) also if you are interested in running tests in an isolated temporary directory.

…scripts

wilzbach · 2018-03-25T08:30:46Z

Yes, I'm aware and I don't have plan working on this in the near future though I might look into replacing the bash scripts with D scripts to please the people on Windows, but I don't use/have Windows, so that's not super-high on my priority list.

(anyhow merge conflicts resolved)

marler8997 · 2018-03-26T15:03:58Z

The big disadvantage I see with "D" is it could make the tests suite take a bit longer since D takes a while to compile when compared to the time it takes to interpret a BASH script. But having to run BASH in Windows is something I haven't been able to get working.

wilzbach force-pushed the bash-dir branch from 97198c3 to fda02dc Compare March 15, 2018 14:33

wilzbach commented Mar 15, 2018

View reviewed changes

wilzbach force-pushed the bash-dir branch 3 times, most recently from 6a476f4 to e3bc75a Compare March 15, 2018 15:03

marler8997 reviewed Mar 15, 2018

View reviewed changes

wilzbach force-pushed the bash-dir branch from e3bc75a to d0b49ea Compare March 15, 2018 18:14

wilzbach changed the title ~~Add and use RESULTS_{TEST_DIR,OUTPUT} + EXTRA_FILES in the bash scripts~~ Add and use RESULTS_TEST_DIR + OUTPUT_BASE + EXTRA_FILES in the bash scripts Mar 15, 2018

wilzbach force-pushed the bash-dir branch 2 times, most recently from 2765d3e to 2631c41 Compare March 15, 2018 18:59

wilzbach added the Review:WIP Work In Progress - not ready for review or pulling label Mar 15, 2018

wilzbach force-pushed the bash-dir branch 2 times, most recently from 9800c01 to 5bd858e Compare March 15, 2018 20:35

marler8997 approved these changes Mar 15, 2018

View reviewed changes

wilzbach force-pushed the bash-dir branch from 5bd858e to 5cd659f Compare March 16, 2018 05:47

wilzbach removed the Review:WIP Work In Progress - not ready for review or pulling label Mar 16, 2018

wilzbach mentioned this pull request Mar 16, 2018

Add a convenience script for running individual tests #8040

Merged

wilzbach force-pushed the bash-dir branch 8 times, most recently from 07a3eff to 76734bd Compare March 22, 2018 21:05

Add and use RESULTS_TEST_DIR + OUTPUT_BASE + EXTRA_FILES in the bash …

42e3d27

…scripts

wilzbach force-pushed the bash-dir branch from 76734bd to 42e3d27 Compare March 25, 2018 08:23

wilzbach mentioned this pull request Mar 25, 2018

Consolidate information on running the testsuite in a README #8079

Merged

wilzbach changed the title ~~Add and use RESULTS_TEST_DIR + OUTPUT_BASE + EXTRA_FILES in the bash scripts~~ Add and use RESULTS_TEST_DIR + OUTPUT_BASE + EXTRA_FILES in the bash scripts of DMD's testsuite Mar 25, 2018

wilzbach mentioned this pull request Mar 25, 2018

Add advice on how to test a PR locally dlang/dlang-bot#194

Merged

WalterBright approved these changes Mar 26, 2018

View reviewed changes

WalterBright merged commit df4a634 into dlang:master Mar 26, 2018

wilzbach deleted the bash-dir branch July 1, 2018 23:26

Uh oh!

Conversation

wilzbach commented Mar 15, 2018

Uh oh!

dlang-bot commented Mar 15, 2018 • edited by wilzbach Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bugzilla references

Testing this PR locally

Uh oh!

wilzbach commented Mar 15, 2018

Uh oh!

wilzbach Mar 15, 2018

Choose a reason for hiding this comment

Uh oh!

wilzbach Mar 15, 2018

Choose a reason for hiding this comment

Uh oh!

wilzbach Mar 15, 2018

Choose a reason for hiding this comment

Uh oh!

wilzbach Mar 15, 2018

Choose a reason for hiding this comment

Uh oh!

wilzbach Mar 15, 2018

Choose a reason for hiding this comment

Uh oh!

marler8997 Mar 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wilzbach Mar 15, 2018

Choose a reason for hiding this comment

Uh oh!

marler8997 commented Mar 15, 2018

Uh oh!

wilzbach commented Mar 22, 2018

Uh oh!

ibuclaw commented Mar 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wilzbach commented Mar 25, 2018

Uh oh!

marler8997 commented Mar 26, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dlang-bot commented Mar 15, 2018 •

edited by wilzbach

Loading

marler8997 Mar 15, 2018 •

edited

Loading

ibuclaw commented Mar 25, 2018 •

edited

Loading