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

[Jacek Caban]

On 6/14/11 4:48 PM, Frédéric Delanoy wrote:
> 2011/6/14 Jacek Caban<jacek at codeweavers.com>:
>> 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?
> see below
>
>>>   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.
> I agree. And the "ok(0, ...)" you wrote some time ago don't look less
> hackish IMHO ;).

It does IMHO, ok(0, ...) is not no-op, ok(TRUE, ...) is.

> The ok(TRUE,...) is used so that, when you have 123 and you expect
> @todo_wine at 123, you get warned about it  ("Test succeeded inside todo
> block").

todo_wine ok(TRUE, ...) is not the same as ok(TRUE, ...). As I said, it 
makes sense, it's just hackish and if it's easily avoidable, why not 
avoid it.

> Without it, I couldn't find a way to get it working.
> I may be incredibly stupid, but I think I still don't get what changes you want:
>
> Do you want this?

No, that doesn't make sense.

>          if(err == out_nl) {
>              if (!is_todo_wine)
>                  ok(!err, "unexpected end of line %d (got '%.*s',
> wanted '%.*s')\n",
>                     line, (int)(out_nl-out_ptr), out_ptr,
> (int)(exp_nl-exp_ptr), exp_ptr);
>              else
>                  todo_wine
>                  ok(!err, "unexpected end of line %d (got '%.*s',
> wanted '%.*s')\n",
>                     line, (int)(out_nl-out_ptr), out_ptr,
>                     (int)(exp_nl-exp_ptr-sizeof(todo_wine_cmd)),
> exp_ptr+sizeof(todo_wine_cmd));
>          }else if(err) {
>              if (!is_todo_wine)
>                  ok(!err, "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(!err, "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));
>          }else {
>              if(!is_todo_wine)
>                 ok(!err, "match at line %d\n", line);
>              else
>                 todo_wine ok(!err, "match at line %d\n", line);
>          }
>
> Or did I misunderstand in
>
> +        }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."
>
> the "make this else unconditional" to mean "remove the if(err)", and
> "to avoid having ok(TRUE, ...) tests late" to mean "delete the last
> else clause containing the 'ok(TRUE, ...)'  calls, and remove the c" ?

Yes, I meant to remove if(!err), make ok calls inside this else block 
ok(!err, ...) and remove the other else block you've added that 
contained ok(TRUE, ...).

Jacek