Den 2011-06-06 11:06 skrev Stefano Lattarini: >> [SNIP] Ditto
>>>> and the reason is a dubious argument to increase testsuite coverage, >>>> see more below. >>>> >>>> We both think the testsuite should execute the script as they are executed >>>> when they are used. And I think that it should only involve $SHELL if there >>>> is a guarantee that $SHELL will always be used, otherwise there will be >>>> inconsistencies. >>>> >>>> It should also be safe to copy-paste things to the command line and >>>> re-run commands (i.e. $SHELL has to be expanded when displayed, I don't >>>> know if this is already the case). >>>> >>> Yes, that should be the case when `xtrace' is in effect: >>> $ sh -c 'SHELL=ksh; set -x; :; $SHELL -c ":"' >>> + : >>> + ksh -c : >> >> Oh, I meant that it should be drop-dead easy to copy-paste from the >> normal build output when you want to re-run some (failing) part of >> the build process (with tweaked arguments), without worrying about >> if $SHELL is set in the environment. Again, that may very well already >> be the case. >> > In the tests I've touched, yes, since they just use $SHELL directly to > run the checked script; and for what concerns these checked scripts, > doing: > $ export SHELL=/bin/bash; $SHELL ar-lib ... > or: > $ /bin/bash ar-lib ... > should be perfectly equivalent. I'm probably having a hard time expressing my concern since there is no problem. It's also not related to this patch, so I'm dropping it. >> >> [SNIP] >> Ditto >> leading to different test results. Is that acceptable? >> > Hardly so; but luckily, it doesn't happen in the present setup :-) Good, thanks for explaining. >>> Would this scenario be more acceptable for you? FWIW, I think it would >>> be an improvement both over the previous situation and my previous patch. >> >> Yes, that would be fine. I thought about that myself, but didn't mention >> it because I'm not too fond of "sister tests"; code duplication makes me >> suspicious. >> > I plan to avoid code duplication as follows: the test code should be > moved in a new *.sh file, so that the sister tests could then just run > by sourcing that file after setting one or two variables that will > configure its behaviour (for a reference, similarly to what is currently > done in master with the auto-generated `instpc*.test' tests). > >> Maybe the affected tests could somehow re-exec themselves >> (only if /bin/sh != $SHELL) with some knob to trigger the sibling behavior >> (I don't know if that is reasonable, just throwing out an idea). That way >> there would be less chance of future errors when updating the tests. >> > I think my idea above is even simpler, and should address your (justified) > concerns about code duplication. Anyway, I'll let you judge my idea when > I have a patch to back it up. Yes, that's a workable approach, I'll have a look at the actual patches. Cheers, Peter
