cmd: add support for todo_wine constructions in testing infrastructure

Frédéric Delanoy frederic.delanoy at gmail.com
Mon Jun 13 04:21:23 CDT 2011


2011/6/13 Dan Kegel <dank at kegel.com>:
> I agree @todo_space@ should go; it's never used, so it's dead code now.
> Tempting to say that removing it should be a separate patch,
> rather than combining that with adding a new feature.

Well, maybe but I didn't see its usefulness when @todo_wine@ is
introduced. It could help for regression testing, admittedly

> Since all the @keywords@ are poorly documented,
> I'm glad to see an unused one go.
>
> Your generic @todo_wine@ keyword seems useful.
>
> The current code is a bit ugly.  If you're going to add a
> nice is_todo_wine_line() helper, it might make sense
> to do a cleanup patch that cleans up the @keyword@
> recognition in general first.  (Why the verbose array
> declaration instead of a "constant"?  etc.)

Those keywords can appear anywhere on the line and act basically as a
search&replace (except @or_broken@) while the @todo_wine@ must be used
at the start of a line to be recognized, and doesn't generate any
text.
But OK an array seemed odd to me as well... it was probably done so
that sizeof(foo_cmd) could be used in memcp(expected_ptr, foo_cmd,
sizeof(foo_cmd)).

Maybe it was done to optimise stuff since sizeof(foo_cmd) would be
computed at compile time, but I don't see why strlen() would not be
for static string constants.

As for the helper, I originally thought about adding  a param to
compare_line so that

static const char *compare_line(const char *out_line, const char
*out_end, const char *exp_line,  const char *exp_end)

becomes

static const char *compare_line(const char *out_line, const char
*out_end, const char *exp_line,  const char *exp_end, BOOL*
is_todo_line)

There would then be no need of a is_todo_wine_line() helper. Would
that be better?

> I see you're making every line of the .exp file count as
> a test.  That seems ok.

That's actually how it currently works: I didn't change that
behaviour, but added a check fo
> There's a fair bit of code duplication; I wonder if you
> could avoid that by refactoring things a bit.

Maybe but I wanted to keep this patch as
> To avoid adding dead code, it might be nice to
> see a test that actually uses your new keyword.

I've made several mkdir tests with it, and it seems to work OK. I was
waiting to see if this patch is commited before I submit.
You can see it here: http://pastebin.com/tnKY4Q7S

>Dan

Frédéric



More information about the wine-devel mailing list