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