Hema,
STATUS
------
Changes and clarifications requested.
REQUESTS
--------
A couple of things:
- I never received a commit email for this patch. Please ensure your commits
are generating the proper emails.
- Why is the backup_views test being deleted? Please take that out of the
patch.
- The backup_partition test needs clarification. Why are character set
specifics included?
- The backup_datatypes test is incomplete and also needs clarification. Why
character set specifics?
- Running tests against all storage engines? Why?
COMMENTARY
----------
I don't think we need to combine special character testing in the partitions
test. That doesn't make sense to me. If we were testing characters as part
of the partitions then perhaps -- but that is a test of the partition
feature, not backup. What exactly are you testing in the partition test? If
it is just that backup can backup and restore a partition table then please
remove the special character processing. It isn't needed and adds
unnecessary complexity.
Likewise, I don't think we need to combine special character testing in the
datatypes test. Besides, it is just the table and database names that use
the characters. That isn't a complete test and doesn't belong in a test that
is (seems to be) designed to test how backup can handle different data
types. In that case, the test is incomplete because it doesn't contain all
possible data types.
I think we need only one (1) test that tests character set use in backup. We
do not need to include it as an agenda for all tests. One comprehensive test
of all of the backup features using mixed character sets (perhaps just one)
is enough. Let's not make testing more complicated by making a character set
emphasis for all tests.
Please clarify what these tests are really testing.
Lastly, please explain why these tests need to be run through all of the
storage engines. I can make a case for the partition test (but I want to
hear your reason), but why datatypes? Storage of data at that level is
beyond the backup system and therefore unnecessary.
On that subject, I see the tests are being put in the backup suite. Does
that mean all tests in that suite will be run against all engines? Did we
not agree to make a special suite for that? Something like
backup_all_engines or some such? I think the backup suite should contain the
basic set of backup tests (e.g.., backup, backup_backupdir, backup_security,
etc.). Special tests like those run against all storage engines should be in
a separate suite.
SUGGESTIONS
-----------
Please use detailed explanations in the per-file comments for your commits.
This will help explain things in greater detail.
DETAILS
-------
Note: Details missing because I could not find the commit email for this
patch.
> > I modified the existing test cases for
> WL#4227(backup_partitions.test
> > and backup_datatypes.test) in accordance to the suggestions
> of you and
> > Bar and made the following modifications:
> >
> > 1) The tests are shifted from backup_charsets suite to backup suite.
> > 2) The test cases will be executed only for all storage
> engines (will
> > not be executed for all character set / collation combinations).
> > 3) I have randomly used accented letters and character sets
> to perform
> > testing. This will ensure that backup stores identifiers
> and objects
> > properly in utf8 format and data is retreived during
> restore without
> > changing the client character set.
> >
> > http://lists.mysql.com/commits/54170
> >
> > There are 2 more test cases for this WL(backup_accented.test and
> > backup_special_characters.test). These tests fail
> sporadically because
> > of Bug#38363. I have attached the patch for these test cases in the
> > bug report, so that developer who is working on this will
> eventually
> > push the tests case once they are fixed.
