List:Commits« Previous MessageNext Message »
From:Hema Sridharan Date:July 1 2008 6:25am
Subject:RE: WL#4222 Test patch review
View as plain text  
Hi,

Please see my comments inline.

> Also, BUG#35249 is 'patch queued' so you must remove the code 
> in the triggers test that disables the memory run.
> 
> > --replace_result $ENGINE ENGINE
> > eval SELECT '$ENGINE'='memory';
> > if(`SELECT '$ENGINE'='memory'`)
> > {
> >  skip Temporarily Disabled Bug#35249 ; }
> 
> One more thing... I see that it is now impossible to run 
> these tests on a system that has only MyISAM built. The tests 
> are designed so that one has to have innodb, falcon, myisam, 
> and memory storage engines built/installed in order to run 
> the tests. I wonder if that is such a good idea?
> 

Hema: It is always possible to remove the engines from the combinations file
and run the tests with the specific engine types.I don't see the necessity
of removing have_falcon and have_innodb. Please advice and I push the patch
once I get your approval.

Thanks,
Hema.S

> I think maybe you need to remove the have_falcon, 
> have_innodb, etc. and handle it like you do above with the 
> engines thing. Only please make a more informative message. 
> 'Temporarily disabled' sounds like it is disabled as in 
> disabled.def file but it isn't. In this case, you should 
> print a message like 'Test skipped. No InnoDB support.'
> 
> Chuck
> 
> 
> --
> 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