[2/3] cmd/tests: Add support for todo_wine constructions (try 2)

[Jacek Caban]

On 6/14/11 3:04 PM, Frédéric Delanoy wrote:
> 2011/6/14 Jacek Caban<jacek at codeweavers.com>:
>> On 6/14/11 10:55 AM, Frédéric Delanoy wrote:
>>> 011/6/14 Jacek Caban<jacek at codeweavers.com>:
>>>> Hi Frédéric,
>>>>
>>>> On 6/13/11 10:19 PM, Frédéric Delanoy wrote:
>>>>> +        }else if(err) {
>>>>> +            if (!is_todo_wine)
>>>>> +                ok(0, "unexpected char 0x%x position %d in line %d (got
>>>>> '%.*s', wanted '%.*s')\n",
>>>>> +                   *err, (int)(err-out_ptr), line,
>>>>> (int)(out_nl-out_ptr),
>>>>> +                   out_ptr, (int)(exp_nl-exp_ptr), exp_ptr);
>>>>> +            else
>>>>> +                todo_wine
>>>>> +                ok(0, "unexpected char 0x%x position %d in line %d (got
>>>>> '%.*s', wanted '%.*s')\n",
>>>>> +                   *err, (int)(err-out_ptr), line,
>>>>> (int)(out_nl-out_ptr),
>>>>> out_ptr,
>>>>> +                   (int)(exp_nl-exp_ptr-sizeof(todo_wine_cmd)),
>>>>> exp_ptr+sizeof(todo_wine_cmd));
>>>> You may change tests to ok(!err, ....) here and make this else
>>>> unconditional
>>>> to avoid having ok(TRUE, ...) tests later.
>>> Which else are you talking about?
>>>
>>> The following ok(TRUE,...) serves when there's a todo_wine which has
>>> "succeeded", to get messages like "Test succeeded inside todo block:
>>> match at line XXX"
>>> Without it (i.e. without at least one "true" OK), successful todos
>>> aren't caught, as Dan Kegel pointed out (and I've verified).
>> I was talking about something like this:
>>
>> }else {
>>     if(!is_todo_wine)
>>         ...
>>     else todo_wine
>>         ok(!err, ...);
>> }
>>
>> If the tests succeeded, err will be NULL, so this ok will produce "Test
>> succeeded inside todo block" error.
> I tried what you say but it didn't work: the test runner doesn't even
> go that far, and doesn't detect any todo, just says something like
>
> batch: 44 tests executed (0 marked as todo, 0 failures), 0 skipped.
>
> instead of the>  100 tests (with a couple of todos for the mkdir
> patch) it should make.
>
> Simply replacing "ok(0,...)" and "ok(TRUE,...)" by "ok(!res,...)"
> works,

Sure it works. But... what were your previous sentences about?

>   but is not more clear IMHO

ok(TRUE, ...) simply doesn't make sense. All we care is that tests don't 
fail. A number of succeeded tests doesn't matter and ok(TRUE, ...) is 
just an useless expression. todo_wine ok(TRUE, ...) makes some sense, 
but still, it looks hackish, so I'd rather avoid it.

> Anyway, the current test runner code is a bit fragile/ugly at times
> right now, and would need some refactoring (e.g. to factor out
> @keyword@ expansion, give better error messages,...) but that is
> probably out of scope of this patch series.

I don't see what's so ugly about it. Factoring out @keyword@ expansions 
won't make things any cleaner, IMO. Parsing code is so simple here that 
I wouldn't make a big deal out of it. Error messages don't matter much - 
after all you should never see them. Perhaps printing the whole failed 
line may help writing tests a bit, but that's just one trivial change here.

Jacek