From: Chuck Bell
Date: July 21 2008 7:24pm
Subject: RE: WL#4222 Test patch review
List-Archive: http://lists.mysql.com/commits/50143
Message-Id: <092886AF6EEE46DD9B0473030FC85342@mysqlcabdesk>
MIME-Version: 1.0
Content-Type: text/plain;
	charset="us-ascii"
Content-Transfer-Encoding: 7bit

Hema,

I'm sorry, but this isn't what I was asking. Take a moment to look at the
test results I just got with a build on Windows that included InnoDB but not
Falcon:

 
TEST                           RESULT         TIME (ms)
-------------------------------------------------------

main.backup                    [ pass ]           2122
main.backup_charsets           [ pass ]            280
main.backup_commit_blocker     [ pass ]           1170
main.backup_commit_restore     [ pass ]           2449
main.backup_compression        [ pass ]            624
main.backup_ddl_blocker        [ pass ]          10390
main.backup_errors             [ pass ]            780
main.backup_fkey               [ pass ]            780
main.backup_lock_myisam        [ pass ]           3510
main.backup_many_dbs           [ pass ]           3510
main.backup_multi_blocks       [ pass ]            312
main.backup_myisam1            [ pass ]            234
main.backup_myisam2            [ pass ]            624
main.backup_no_be              [ disabled ]  Bug#38023 2008-07-16 rafal Test
tri
ggers valgrind warnings described in the bug
main.backup_no_data            [ pass ]           1046
main.backup_no_engine          [ disabled ]  Bug#36021 2008-04-13 rsomla
server
crashes when openning table with unknown storage engine
main.backup_nodata_driver      [ fail ]

mysqltest: At line 130: query 'RESTORE FROM 'bup_nodata.bak'' failed: 1146:
Table 'bup_data.myisam1' doesn't exist

Hema: ignore this failure. It is unrelated to your patch.

...

Resuming Tests

main.backup_objects            [ pass ]           1123
main.backup_procedures         [ pass ]            218

Wait! Why is this test here? Isn't there a backup_procedures test in the
backup suite? Shouldn't we remove this one?

main.backup_progress           [ pass ]            562
main.backup_security           [ pass ]            483
main.backup_snapshot           [ pass ]           1030
main.backup_tablespace         [ disabled ]  Bug#36973 2008-07-01 rafal
main.backup_triggers_and_events [ disabled ]  Bug#37762 2008-07-01 rafal
Test fa
ils on remove_file for unknown reasons
main.backup_view_on_view       [ pass ]            328
main.backup_views              [ pass ]           2168
backup.backup_functions 'myisam' [ pass ]           1248
backup.backup_online_testing 'myisam' [ pass ]           2761
backup.backup_procedures 'myisam' [ pass ]            546
backup.backup_triggers 'myisam' [ pass ]           1826
backup.backup_functions 'falcon' [ pass ]           1061
backup.backup_online_testing 'falcon' [ pass ]           2730
backup.backup_procedures 'falcon' [ pass ]            530
backup.backup_triggers 'falcon' [ pass ]           1716

No, this is wrong. I did not have Falcon compiled. No Falcon tests should
run here. This is a false positive test result. The result should say:

   backup.backup_... 'falcon' [ skipped ]  Requires falcon storage engine

...or some such.

backup.backup_functions 'memory' [ pass ]            734
backup.backup_online_testing 'memory' [ pass ]           2839
backup.backup_procedures 'memory' [ pass ]            514
backup.backup_triggers 'memory' [ pass ]           1201
backup.backup_functions 'innodb' [ pass ]            983
backup.backup_online_testing 'innodb' [ pass ]           2792
backup.backup_procedures 'innodb' [ pass ]            530
backup.backup_triggers 'innodb' [ pass ]           1763
ndb.ndb_alter_table_backup     [ skipped ]   No ndbcluster support
-------------------------------------------------------
Stopping All Servers
Failed 1/38 tests, 97.37% were successful.

I do not mean to be so unrelenting in my review demands, but I feel strongly
these tests can be written to run IFF the storage engine is present and that
they should not say 'pass' when they haven't run at all. 

Chuck