maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxxx>]

Hi, Kristian!

On Aug 05, Kristian Nielsen wrote:
> 
> Sergei Golubchik <serg@xxxxxxxxxxxx> writes:
> 
> >> Other than that I think it is ok to push. I have a number of small remarks
> >> inline below, you can fix what you agree with and ignore the rest. There are
> >> also a couple of questions to check that my understanding is correct.
> >
> > Now, that you've seen the patch - should I push it in 5.1 too ?
> 
> I weakly suggest "yes".
> 
> The patch does not introduce any interface changes to mysql-test-run.pl, so
> probably this one is less important to have identical in all versions.
> 
> But it seems to me still preferable to have mysql-test-run as identical as
> possible between versions, and I don't see that it could hurt.
> 
> >> Well, the dots '.' in the keys of %servers will need to be \escaped, or they
> >> will match any char, not just '.'.
> 
> > I suppose so. But the old code was not doing it.
> 
> Ok. (maybe it's not important, lots of Perl code does this and it
> usually works as it's not likely that we will ever see a string that
> wrongly matches due to '.' rather than '\.'. Since it is a common
> mistake, I tend to look out for it).

I know, it catched my eye too, but I preferred to left the old behavior.
I'll change it to be "correct" now, and if the test suite will pass I'll
keep it.

> >> > === added file 'mysql-test/suite/sphinx/sphinx.test'
> >> > --- mysql-test/suite/sphinx/sphinx.test	1970-01-01 00:00:00 +0000
> >> > +++ mysql-test/suite/sphinx/sphinx.test	2010-08-02 20:35:41 +0000
> >> > @@ -0,0 +1,25 @@
> >> > +if (`SELECT "$HAVE_SPHINX" < "0000.0009.0009"`) {
> >> > +   skip Need Sphinx version 0.9.9 or later;
> >> > +}
> >> 
> >> There is no --include have_shpinx_storage_engine_plugin.inc.
> >> 
> >> Do I understand correctly that this is because the presense of --plugin-load
> >> in suite.opt automagically disables the suite if the plugin is not available?
> >
> > No, because I've added the contents of a
> > have_shpinx_storage_engine_plugin.inc directly into sphinx.test file.
> 
> I think you misunderstood me. This check (`SELECT "$HAVE_SPHINX" <
> "0000.0009.0009"`) is for the version of the external sphinx binaries. But we
> also need to skip the test case if the sphinx storage engine is not built.

Oh, right. I'll fix it.
 
Regards,
Sergei