List:Commits« Previous MessageNext Message »
From:Chuck Bell Date:June 30 2008 8:42pm
Subject:RE: WL#4222 Test patch review
View as plain text  
Hema,

Hold on a minute. I just saw that random error in another test run unrelated
to your patch. Please ignore that and address my other comments: 

> 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?
> 
> 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

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