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

Thanks for your comments. Please see my replies inline.

> Hi !
> 
> 
> Let me join here:
> 
> Chuck Bell wrote:
> > 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.
> 
>  From a build team point of view, I very strongly support that:
> 
> It is mandatory for us that exactly those tests are run which 
> check components (table handlers, collations, ...) configured 
> in the build, and tests for other components get skipped.
> 
> For examples of this, please see the tests written by 
> Matthias Leich in the "funcs1" suite, whose result files 
> differ by the engine tested:

Hema: I went through the funcs_1 suite and noticed a separate test/result
file for each storage engine, testing the same functionality. Unfortunately
this was not approved by my team when I followed exactly the same method
sometime ago. They wanted me to have common test/result file for different
storage engines which inturn will not duplicate the test and result file. 

For this reason I used something called "combinations" file, which will have
list of storage engines listed in it. Then each test from the suite will run
for all the storage engines listed in the combinations file.
Note: Just FYI, this mechanism can be seen currently in rpl suite where each
test runs for all replication formats.



>     {innodb,memory,myisam,ndb}_{func_view,storedproc,views}.test
> (all in "mysql-test/suite/funcs_1/t/").
> 
> In these tests, you should see the mechanism used to skip 
> (controlled by the presence of a feature) and to write a 
> meaningful comment into the output (which is in the release 
> build log we keep archived).


Hema: Skip could be because of couple of reasons 
1. Skip because of a defect pertinent to a specific engine.(I believe that
this is what you are referring)
2. Skip the tests based on storage engines compiled in the Build.(I'm
implementing this option also)

> 
> 
> For another way of skipping, controlled by the comment string 
> which we set in the release build, see the 
> "charset_collation_*.test" files in the same directory.
> 
> > 
> IMNSHO, the silent replacement of one storage engine by 
> another is a perfect way to make tests fail.
> In production environments, it might make the system behave 
> in an un-expected and un-intended way, which may cause wrong 
> data and so be a bad failure for the user/customer.
> For a recent example, see my report of bug#37976.

Hema: From the bug report I could see that silent replacement of MyISAM for
Innodb in slave is probably the
expected behavior because of --loose-innodb option. By using loose option,
MySQL Server will ignore the option if it's not supported. Maybe you should
try in slave without --loose-innodb option and with less disk space.

Let me know if you have further questions.

Thanks,
Hema.S


> 
> I would rather have the system throw an error if a request 
> (say, to use a certain table handler) cannot be fulfilled 
> than have it continue with changed semantics (say, without 
> transactions).
> 
> 
> Regards,
> Jörg
> 
> -- 
> Joerg Bruehe,  MySQL Build Team,  joerg@stripped   (+49 30) 
> 417 01 487
> Sun Microsystems GmbH,   Sonnenallee 1,   D-85551 
> Kirchheim-Heimstetten
> Geschaeftsfuehrer: Thomas Schroeder, Wolfgang Engels, Dr. 
> Roland Boemer
> Vorsitzender des Aufsichtsrates: Martin Haering     Muenchen: 
> HRB161028
> 
> 

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