shell32/tests: add tests for the parser of SHELLEXECUTEINFO.lpFile [try 6]

Ilya Basin basinilya at gmail.com
Mon Mar 8 11:25:56 CST 2010


PV> On 03/08/2010 05:42 PM, Juan Lang wrote:
>> Hi Ilya,
>> I like this one rather better, thanks.  Especially the use of broken()
>> makes it clearer what's happening.
>>
>> I think you could tidy it up just a little more:
>>
>>> +    expected = 33; sprintf(fileA, testfile, tmpdir);
>>> +    rc=shell_execute(NULL, fileA, NULL, NULL);
>>> +    todo_wine {
>>> +        ok(rc==expected || (rc>32&&  expected>32),
>>> +            "expected %s (%d), got %s (%d), lpFile: %s \n",
>>> +            expected==33 ? "success" : "failure", expected, rc>  32 ? "success" : "failure", rc, fileA
>>> +            );
>>> +    }
>>
>> In this case, expected is 33, so the "expected == 33" is unneeded.
>> Just replace "%s (%d)" with "success (32)".  Same below with the
>> remaining ok expressions:  make them match the actual value of
>> expected, rather than any possible value.  Otherwise, looks good to
>> me.

PV> Yep, I like this one better as well. The only thing I see is that you 
PV> have "==expected" and "rc>32" (which is both true in the above case). Is 
PV> that to cover for OS differences? If so, please add some comments.
No, that's because I use the same expression for both failure and
success.
Now Juan tells me to have unique format string for each case. Let's stop
here already! I'm sure all of us are unable to predict what exactly
the guy with the commit power won't like in my patch. Quit taking
precautions, please.

-- 




More information about the wine-devel mailing list