maria-developers team mailing list archive
Mailing list archive
Re: Review of mysql-test-run speedup patch
Sergei Golubchik <serg@xxxxxxxxxxxx> writes:
> Just one question and couple of suggestions below.
> If you agree with them - please change accordingly and push!
Thanks for review! I believe I addressed all your suggestions.
>> + $base_dir= dirname($file)
>> + unless defined($base_dir);
> why do you pass base_dir as an argument, instead of always using
> basedir($file) ?
It was a mistake. I was trying to preserve old behaviour, but misunderstood
what that behaviour was. Changed to use basedir($file) always.
>> + for my $sourced_file ($base_dir . '/' . $include,
>> + $::glob_mysql_test_dir . '/' . $include)
> Can you also look in the suite dir ? In the order of
> basedir(file), suite_dir, glob_mysql_test_dir.
>> + my ($sub_tags, $sub_master_opts, $sub_slave_opts)=
>> + get_tags_from_file($sourced_file, $base_dir);
>> + push @$tags, @$sub_tags;
>> + # Tests (rpl.rpl_ddl at least) rely on options being in reverse order
>> + # of include :-/ Hence the unshift().
>> + unshift @$master_opts, @$sub_master_opts;
>> + unshift @$slave_opts, @$sub_slave_opts;
> I think having options in the order of includes is more logical.
> Better to sort includes in the rpl_ddl.test, and use push here.
> It is more intuitive and less error-prone.
It turns out what is really needed is that options from includes are added
before options from main file, so main file can override options from includes
(this is what rpl_ddl needs, include/have_innodb adds --innodb, then
rpl_ddl.test adds --skip-innodb for the slave only).
So I changed it so that options come in order of includes, but first from
included files, last from including file. More logical indeed.