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.
Jacek
011/6/14 Jacek Caban jacek@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).
On 6/14/11 10:55 AM, Frédéric Delanoy wrote:
011/6/14 Jacek Cabanjacek@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.
Jacek
2011/6/14 Jacek Caban jacek@codeweavers.com:
On 6/14/11 10:55 AM, Frédéric Delanoy wrote:
011/6/14 Jacek Cabanjacek@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, but is not more clear IMHO
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.
Frédéric
On 6/14/11 3:04 PM, Frédéric Delanoy wrote:
2011/6/14 Jacek Cabanjacek@codeweavers.com:
On 6/14/11 10:55 AM, Frédéric Delanoy wrote:
011/6/14 Jacek Cabanjacek@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
2011/6/14 Jacek Caban jacek@codeweavers.com:
On 6/14/11 3:04 PM, Frédéric Delanoy wrote:
2011/6/14 Jacek Cabanjacek@codeweavers.com:
On 6/14/11 10:55 AM, Frédéric Delanoy wrote:
011/6/14 Jacek Cabanjacek@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 ;). The ok(TRUE,...) is used so that, when you have 123 and you expect @todo_wine@123, you get warned about it ("Test succeeded inside todo block").
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?
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" ?
I must admit I'm a bit confused
Frédéric
On 6/14/11 4:48 PM, Frédéric Delanoy wrote:
2011/6/14 Jacek Cabanjacek@codeweavers.com:
On 6/14/11 3:04 PM, Frédéric Delanoy wrote:
2011/6/14 Jacek Cabanjacek@codeweavers.com:
On 6/14/11 10:55 AM, Frédéric Delanoy wrote:
011/6/14 Jacek Cabanjacek@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@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