Hiya,<div><br></div><div>Thanks for the comments</div><div><br></div><div><span style="font-family:arial,sans-serif;font-size:13.333333969116211px">>I don't really like hardcoding a variable name in a function... </span><br>
</div><div><span style="font-family:arial,sans-serif;font-size:13.333333969116211px">I don't mind not having the variable hardcoded in the function, but I dont like the solution(s), and after all they are only test routines!</span><span style="font-family:arial,sans-serif;font-size:13.333333969116211px"> One extremely useful benefit of naming the variable and checking the expected contents rather than just echoing them, is that you can extract the chunk of the test script into a standalone testcase and run/debug that on whatever o/s without needing to go through the test infrastructure - whereas getting output from the command script and not knowing whether it is right or wrong until you go off and compare it to some hard coded list of values is much harder to work with.</span></div>
<div><span style="font-family:arial,sans-serif;font-size:13.333333969116211px"><br></span></div><div><font face="arial, sans-serif">An interim option might be something like 'checkcontents var value var value' type routine - would that be better?</font></div>
<div><br></div><div><span style="font-family:arial,sans-serif;font-size:13.333333969116211px">>2. The check for the presence of the first parameter is unneeded since</span><br style="font-family:arial,sans-serif;font-size:13.333333969116211px">
<span style="font-family:arial,sans-serif;font-size:13.333333969116211px">>you always call with at least one param (you control all calls)</span></div><div><br></div><div>Its just an example of defensive coding, for which I make no excuse :-)<br>
</div><div><br></div><div><span style="font-family:arial,sans-serif;font-size:13.333333969116211px">>3. When only 1 variable is used, no need to add a '1' suffix as in</span><br><span style="font-family:arial,sans-serif;font-size:13.333333969116211px">>'foo1' or 'WINE_foo1': the '1' doesn't help/add useful information</span></div>
<div><br></div><div>No, but it doesnt hurt either and gives consistency with the other tests<br style="font-family:arial,sans-serif;font-size:13.333333969116211px"><span style="font-family:arial,sans-serif;font-size:13.333333969116211px"><br>
</span></div><div><font face="arial, sans-serif">If the patch doesnt go in as-is, I'll recode the check routine using what I describe above and if it does, I'll just patch the routine... If its a generic routine as described above it can then be reused elsewhere in the tests potentially as well</font></div>
<div><font face="arial, sans-serif"><br></font></div><div><font face="arial, sans-serif">Thanks for the review!</font></div><div><font face="arial, sans-serif">Jason</font></div><div class="gmail_extra"><br><br><div class="gmail_quote">
On 14 December 2012 02:01, Frédéric Delanoy <span dir="ltr"><<a href="mailto:frederic.delanoy@gmail.com" target="_blank">frederic.delanoy@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">On Fri, Dec 14, 2012 at 2:59 AM, Frédéric Delanoy<br>
<<a href="mailto:frederic.delanoy@gmail.com">frederic.delanoy@gmail.com</a>> wrote:<br>
> On Thu, Dec 13, 2012 at 10:59 PM, Ann and Jason Edmeades<br>
> <<a href="mailto:jason@edmeades.me.uk">jason@edmeades.me.uk</a>> wrote:<br>
>> The tests previously set a variable which was not checked until<br>
>> the subsequent test completed (as env vars are expanded as the<br>
>> line is read, meaning a sequence of set /a var=x & echo %var%<br>
>> echos the contents before the set is executed). In addition when<br>
>> multiple variables are involved in the calculation, only the<br>
>> first one was being checked, and this is changed to check all<br>
>> variables involved in the calculation.<br>
>><br>
> +<br>
> +REM Check the variables are set to the expected value and clear their contents<br>
> +:check_vars<br>
> +if not "%1"=="" (<br>
> + if "%1"=="%WINE_var1%" (<br>
> + echo WINE_var1 correctly %1<br>
> + ) else (<br>
> + echo ERROR: WINE_var1 incorrectly %WINE_var1% [%1]<br>
> + )<br>
> +)<br>
> +if not "%2"=="" (<br>
> + if "%2"=="%WINE_var2%" (<br>
> + echo WINE_var2 correctly %2<br>
> + ) else (<br>
> + echo ERROR: WINE_var2 incorrectly %WINE_var2% [%2]<br>
> + )<br>
> +)<br>
> +if not "%3"=="" (<br>
> + if "%3"=="%WINE_var3%" (<br>
> + echo WINE_var3 correctly %1<br>
> + ) else (<br>
> + echo ERROR: WINE_var3 incorrectly %WINE_var3% [%3]<br>
> + )<br>
> +)<br>
><br>
> A couple comments:<br>
><br>
> 1. I don't really like hardcoding a variable name in a function... Why<br>
> not using something like:<br>
> set /a var=1 +2 & call :compute var<br>
> set /a foo=8, bar=foo+1 & call :compute foo bar<br>
><br>
> goto :end<br>
> :compute<br>
<br>
</div></div>Note "compute" is probably a bad name, "display" might be better to<br>
describe the purpose of the routine.<br>
<span class="HOEnZb"><font color="#888888"><br>
Frédéric<br>
</font></span></blockquote></div><br></div>