List:Commits« Previous MessageNext Message »
From:Chuck Bell Date:July 23 2008 1:06pm
Subject:RE: WL#4222 Test patch review
View as plain text  
Hema,

Above all, you need to fix the false positive test result for falcon tests
when no falcon engine exists. This is wrong. This applies to all storage
engines. If they aren't there the test should not say that the test passed.

Chuck 

> -----Original Message-----
> From: Hema Sridharan [mailto:hema@stripped] 
> Sent: Wednesday, July 23, 2008 0:35 AM
> To: 'Chuck Bell'
> Cc: 'Rafal Somla'; 'Lars Thalmann'; commits@stripped
> Subject: RE: WL#4222 Test patch review
> 
> Hi Chuck,
> 
> I created this patch after taking your suggestion in IRC on 
> 07/02. For my proposal , "replacing by default storage 
> engines, if the engines like Falcon(or Innodb) is not 
> compiled in the BUILD" you gave your approval and that's why 
> I proceeded. 
> 
> Also, the separate backup_charsets suite is created based on 
> suggestion from Magnus and Lars( Forwarded the email to you). 
> I also sent an email to dev-backup regarding this query on 
> 06/18 on subject:"Clarification: Backup tests with different 
> character sets and collations".
> 
> If you still need further changes on this, let me know.
> 
> Warm Regards,
> Hema Sridharan
> QA Developer
> Sun Microsystems / Mysql Inc, www.sun.com  www.mysql.com
> Office: Austin TX 78728, USA
>  
> Are you MySQL certified?  www.mysql.com/certification
>  
> 
> > -----Original Message-----
> > From: Chuck Bell [mailto:cbell@stripped]
> > Sent: Monday, July 21, 2008 1:25 PM
> > To: 'Hema Sridharan'
> > Cc: commits@stripped; 'Lars Thalmann'; 'Rafal Somla'
> > Subject: RE: WL#4222 Test patch review
> > 
> > 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
> > 
> > 
> > --
> > MySQL Code Commits Mailing List
> > For list archives: http://lists.mysql.com/commits
> > To unsubscribe:    
> http://lists.mysql.com/commits?unsub=1
> > 
> > 
> 
> 
> --
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe:    
> http://lists.mysql.com/commits?unsub=1
> 

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