Bug #47383 let mysql-test-run.pl run innodb tests using the plugin
Submitted: 16 Sep 2009 19:56 Modified: 6 Apr 2010 21:28
Reporter: Mark Callaghan Email Updates:
Status: Closed Impact on me:
None 
Category:Tools: MTR / mysql-test-run Severity:S4 (Feature request)
Version:5.1.38, 5.1.40 OS:Any
Assigned to: Bjørn Munch CPU Architecture:Any
Tags: Contribution, innodb, mysql-test-run, plugin

[16 Sep 2009 19:56] Mark Callaghan
Description:
When the Innodb builtin is disabled but the plugin is available, I want to be able to run the existing innodb tests. That is not possible today because mysql-test-run uses the output from 'mysqld --help --verbose' to determine what features are available and '--innodb' is not displayed in this case.

But it is available when I run mysql-test-run with the '--mysqld=--innodb' flag. I will attach a patch that modified mysql-test-run.pl to updated ::mysqld_variables for flags passed on the command line so that 'No innodb support' is not printed and the innodb tests skipped when the plugin is enabled.

How to repeat:
./configure --without-plugin-innobase --with-plugin-innodb_plugin
./mysql-test-run.pl --mysqld=--innodb innodb

the test will be skipped with the output 'No innodb support'

Suggested fix:
patch to be attached
[16 Sep 2009 19:59] Mark Callaghan
patch for mysql-test/mysql-test-run.pl

Attachment: mtr (application/octet-stream, text), 1.03 KiB.

[17 Sep 2009 9:31] Valeriy Kravchuk
Thank you for the feature request and patch contributed.
[24 Sep 2009 9:15] Bjørn Munch
I try to follow the "how to repeat" with the patch, and mtr does indeed start the test, but the server fails:

----------------
090924 12:11:47 [Warning] /home/bm136801/my/innoplug-51/sql/mysqld: unknown variable 'loose-innodb_data_file_path=ibdata1:10M:autoextend'
090924 12:11:47 [Warning] /home/bm136801/my/innoplug-51/sql/mysqld: unknown option '--loose-skip-innodb'
090924 12:11:47 [ERROR] /home/bm136801/my/innoplug-51/sql/mysqld: unknown option '--innodb'
090924 12:11:47 [ERROR] Aborting
-----------------
[24 Sep 2009 14:36] Mark Callaghan
I left out a step. Something must be done to link the Innodb plugin by default as described in http://bugs.mysql.com/bug.php?id=47350, or to use the load plugin options to load the plugin shared object.
[29 Sep 2009 10:22] Bjørn Munch
I haven't been able to reproduce your setting with the current 5.1.  If I don't apply the patch from Bug #47350, then mysqld does not accept --innodb and the test fails even if mtr has been patched to run it.

But if I do apply the patch and rebuild, then mysqld accepts --innodb but also reports it has innodb enabled, so the patch is unnecessary; mtr will happily run innodb tests.

Now, mtr has a recent addition which will make it run innodb tests also with the innodb plugin if available. But this will not work if mysqld does not have builtin innodb, because the test will then have been marked as skipped and ignored. What would be useful is to change the logic so that tests that have been skipped due to "No InnoDB support" are included here.
[29 Sep 2009 13:41] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/85010

2837 Bjorn Munch	2009-09-29
      Bug #47383 let mysql-test-run.pl run innodb tests using the plugin
      Alt. solution: let the "InnoDB plugin" combinations apply
      Added some alternative plugin paths (I need to move the code anyway)
[29 Sep 2009 13:45] Bjørn Munch
This isn't the solution the bug reported suggested but it should have the same effect.
[30 Sep 2009 8:39] Magnus Blåudd
Good patch that is local only to mtr_cases.pm!

I would suggest that you use the My::Find module when looking for the files, since it's the modern version of mtr_file_exist and has builtin support for 'vs_config_dirs' etc.
[30 Sep 2009 21:56] Kent Boortz
Looks good. I don't like the idea to match on "comment", will break if someone changes the text a bit, or introduces variations on the "No innodb" comment for different reasons to skip innodb.

A question, did you mean for "return $tinfo unless $do_innodb_plugin;" not to return at all from the function, or did you mean "return ($do_innodb_plugin ? undef : $tinfo);"?
[30 Sep 2009 21:57] Kent Boortz
Looks good. I don't like the idea to match on "comment", will break if someone changes the text a bit, or introduces variations on the "No innodb" comment for different reasons to skip innodb.

A question, did you mean for "return $tinfo unless $do_innodb_plugin;" not to return at all from the function, or did you mean "return ($do_innodb_plugin ? undef : $tinfo);"?
[2 Oct 2009 8:37] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/85481

2837 Bjorn Munch	2009-10-02
      Bug #47383 let mysql-test-run.pl run innodb tests using the plugin
      Alt. solution: let the "InnoDB plugin" combinations apply
      Added some alternative plugin paths (I need to move the code anyway)
[2 Oct 2009 8:38] Bjørn Munch
Second patch follows the suggestion of using My::Find, and also adds a comment to the comment to warn against changing the text.
[2 Oct 2009 12:36] Bjørn Munch
Pushed to 5.1-mtr (upmerged to later-mtr soon)
[22 Oct 2009 20:17] Bugs System
Pushed into 6.0.14-alpha (revid:alik@sun.com-20091022201524-0efl2fzebfuuf0vk) (version source revid:bjorn.munch@sun.com-20091005090144-38admpr88wyaiksr) (merge vers: 6.0.14-alpha) (pib:13)
[22 Oct 2009 20:18] Bugs System
Pushed into 5.5.0-beta (revid:alik@sun.com-20091022201318-jfvtrzd6lb07cwp5) (version source revid:bjorn.munch@sun.com-20091005085600-w29tuifc1iehvy10) (merge vers: 5.4.5-beta) (pib:13)
[22 Oct 2009 23:24] Paul DuBois
Test suite change. No changelog entry needed.

Setting report to NDI pending push into 5.1.x.
[23 Oct 2009 7:33] Bugs System
Pushed into 5.1.41 (revid:bjorn.munch@sun.com-20091021073307-ummbh6668hvfxqjv) (version source revid:bjorn.munch@sun.com-20091021073307-ummbh6668hvfxqjv) (merge vers: 5.1.41) (pib:13)
[23 Oct 2009 15:20] Paul DuBois
Test suite change. No changelog entry needed.
[18 Dec 2009 10:33] Bugs System
Pushed into 5.1.41-ndb-7.1.0 (revid:jonas@mysql.com-20091218102229-64tk47xonu3dv6r6) (version source revid:jonas@mysql.com-20091218095730-26gwjidfsdw45dto) (merge vers: 5.1.41-ndb-7.1.0) (pib:15)
[18 Dec 2009 10:49] Bugs System
Pushed into 5.1.41-ndb-6.2.19 (revid:jonas@mysql.com-20091218100224-vtzr0fahhsuhjsmt) (version source revid:jonas@mysql.com-20091217101452-qwzyaig50w74xmye) (merge vers: 5.1.41-ndb-6.2.19) (pib:15)
[18 Dec 2009 11:04] Bugs System
Pushed into 5.1.41-ndb-6.3.31 (revid:jonas@mysql.com-20091218100616-75d9tek96o6ob6k0) (version source revid:jonas@mysql.com-20091217154335-290no45qdins5bwo) (merge vers: 5.1.41-ndb-6.3.31) (pib:15)
[18 Dec 2009 11:18] Bugs System
Pushed into 5.1.41-ndb-7.0.11 (revid:jonas@mysql.com-20091218101303-ga32mrnr15jsa606) (version source revid:jonas@mysql.com-20091218064304-ezreonykd9f4kelk) (merge vers: 5.1.41-ndb-7.0.11) (pib:15)
[6 Apr 2010 20:56] Mark Callaghan
Can you update the docs for mtr to describe how to test the InnoDB plugin?
[6 Apr 2010 21:12] Bjørn Munch
mtr should in fact run the relevant tests with the InnoDB plugin if it's available, but you're quite right that this is not in the doc. BTW make sure you're reading the doc for mtr "2.0", currently under http://dev.mysql.com/doc/mysqltest/2.0/en/index.html . I overlooked this feature when upgrading the doc because there was no interface change (no new option).
[6 Apr 2010 21:28] Mark Callaghan
Yes, but code has been added to mysql-test/lib/mtr_cases.pm that skips some tests for the InnoDB plugin.

There are better ways to skip tests. If this must be done then this shouldn't reuse an existing message ('No innodb support') that is incorrect in this case.

    foreach my $test (@cases)
    {
      next if (!$test->{'innodb_test'});
      # If skipped due to no builtin innodb, we can still run it with plugin
      next if ($test->{'skip'} && $test->{comment} ne "No innodb support");
      # Exceptions
      next if ($test->{'name'} eq 'main.innodb'); # Failed with wrong errno (fk)
      next if ($test->{'name'} eq 'main.index_merge_innodb'); # Explain diff
      # innodb_file_per_table is rw with innodb_plugin
      next if ($test->{'name'} eq 'sys_vars.innodb_file_per_table_basic');
      # innodb_lock_wait_timeout is rw with innodb_plugin
      next if ($test->{'name'} eq 'sys_vars.innodb_lock_wait_timeout_basic');
      # Diff around innodb_thread_concurrency variable
      next if ($test->{'name'} eq 'sys_vars.innodb_thread_concurrency_basic');
      # Can't work with InnoPlug. Test framework needs to be re-designed.
      next if ($test->{'name'} eq 'main.innodb_bug46000');
      # Fails with innodb plugin
      next if ($test->{'name'} eq 'main.innodb-autoinc');
      # Fails with innodb plugin: r6185 Testcases changes not included
      next if ($test->{'name'} eq 'main.innodb_bug44369');
      # Copy test options
      my $new_test= My::Test->new();
      while (my ($key, $value) = each(%$test))
      {
        if (ref $value eq "ARRAY")
        {
[6 Apr 2010 21:47] Bjørn Munch
I agree, this is not the proper way to skip tests (even if it may be correct to skip them), it should not be hardcoded in mtr itself.

The use of (or reference to) "No innodb support" is however correct as it refers to the reason mtr had for skipping the test *without* innodb plugin. Not very elegant, but correct.