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

My concern is the test will not run at all if you don't have innodb or
falcon. Basically, the test will only run in an environment built as 'max'
-- something that is not likely to happen (IMHO for developers). As a
developer, that means I have to build my system differently and, in the case
of falcon, adds a considerable amount to the build time. 

I think the tests should be able to be run on systems where Falcon or InnoDB
are not included. Please change the tests to allow the tests to run if any
of the engines are available.

Also, please change the message printed by the test. It is not intuitive.

Chuck

> > 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.'

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