List:Commits« Previous MessageNext Message »
From:Chuck Bell Date:July 21 2008 7:24pm
Subject:RE: WL#4222 Test patch review
View as plain text  
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

Thread
WL#4222 Test patch reviewChuck Bell12 Jun
  • RE: WL#4222 Test patch reviewHema Sridharan13 Jun
    • RE: WL#4222 Test patch reviewChuck Bell13 Jun
      • RE: WL#4222 Test patch reviewHema Sridharan24 Jun
        • RE: WL#4222 Test patch reviewChuck Bell30 Jun
          • RE: WL#4222 Test patch reviewHema Sridharan1 Jul
            • RE: WL#4222 Test patch reviewChuck Bell1 Jul
              • RE: WL#4222 Test patch reviewHema Sridharan2 Jul
                • RE: WL#4222 Test patch reviewChuck Bell2 Jul
                  • RE: WL#4222 Test patch reviewHema Sridharan3 Jul
                    • RE: WL#4222 Test patch reviewChuck Bell21 Jul
                      • RE: WL#4222 Test patch reviewHema Sridharan23 Jul
                        • RE: WL#4222 Test patch reviewChuck Bell23 Jul
                          • Re: WL#4222 Test patch reviewJoerg Bruehe31 Jul
                            • RE: WL#4222 Test patch reviewHema Sridharan1 Aug
                              • Re: WL#4222 Test patch reviewJoerg Bruehe1 Aug
                      • RE: WL#4222 Test patch reviewHema Sridharan7 Aug
                        • RE: WL#4222 Test patch reviewChuck Bell11 Aug
                          • RE: WL#4222 Test patch reviewHema Sridharan11 Aug
RE: WL#4222 Test patch reviewChuck Bell30 Jun
  • RE: WL#4222 Test patch reviewHema Sridharan30 Jun