Chuck,
Apart from a few typos, the patch is approved. Great to have all these
errors covered in tests! Good work.
STATUS:
-------
Approved, pending fix of requests.
REQUESTS:
---------
1. Fix typo in comment
2. Fix typo in comment
3. Fix typo in note
4. Is this BUG# correct?
OPTIONAL:
---------
5. Move verbose error injection to separate methods in backup_info.cc
and kernel.cc
DETAILS:
--------
=== added file 'mysql-test/suite/backup/include/test_for_error.inc'
(...)
+# If error number matches the error we are test for the test case passes.
+if (`SELECT $my_err = $errno`)
+{
1. Typo? "... the error we are testing for..."?
=== added file 'mysql-test/suite/backup/t/backup_errors_debug_1.test'
(...)
+# Part 1 of 4 : covers test cases 01 - 23.
+# Part 2 of 4 : covers test cases 24 - 48.
+# Part 3 of 4 : covers test cases 49 - 70.
2. Typo? Part 1/2/3 of 3? (also test files debug_2 and debug_3)
=== added file 'mysql-test/suite/backup/t/backup_errors_debug_2.test'
(...)
+# Note: The error ER_BACKUP_CONTEXT_CREATE masks ER_BACKUP_CONTEXT_CREATE
+#
+# Test for error ER_BACKUP_INTERRUPTED.
+LET $caseno = 37;
+LET $errno = $ER_BACKUP_CONTEXT_CREATE;
+LET $errname = ER_BACKUP_INTERRUPTED;
+LET $operation = BACKUP;
+--source suite/backup/include/test_for_error.inc
3. Typo in Note? It doesn't mask itself?
=== added file 'mysql-test/suite/backup/t/backup_errors_debug_3.test'
(...)
+#
+# Notice: Test case 53 disabled. See BUG#46596.
+#
+# Test case 53 requires running once for backup and once for restore.
+#
+# Test for error ER_BACKUP_SEND_REPLY.
+LET $caseno = 53a;
+LET $errno = $ER_BACKUP_SEND_REPLY;
+LET $errname = ER_BACKUP_SEND_REPLY;
+LET $operation = BACKUP;
+#--source suite/backup/include/test_for_error.inc
4. Is this the correct BUG#?
=== modified file 'sql/backup/backup_info.cc'
(...)
@@ -1211,6 +1241,39 @@ Backup_info::add_db_object(Db &db, const
int res= get_dep_node(db.name(), *obj->get_name(), type, n);
+ /*
+ Debug insertion for errors. Here we setup the error to correspond
+ with the debug SESSION tag specified in the test.
+
+ Note: Error code insertion won't work here as we need to generate
+ the specific error message for each test case.
+
+ Note: ER_BACKUP_CATALOG_ADD_TABLE is detected elsewhere.
+
+ */
+ ::String str;
+ switch (type) {
+ case BSTREAM_IT_DB: str.append("ER_BACKUP_CATALOG_ADD_DB"); break;
+ case BSTREAM_IT_VIEW: str.append("ER_BACKUP_CATALOG_ADD_VIEW"); break;
+ case BSTREAM_IT_SPROC: str.append("ER_BACKUP_CATALOG_ADD_SROUT_P");
break;
+ case BSTREAM_IT_SFUNC: str.append("ER_BACKUP_CATALOG_ADD_SROUT_F");
break;
+ case BSTREAM_IT_EVENT: str.append("ER_BACKUP_CATALOG_ADD_EVENT"); break;
+ case BSTREAM_IT_TRIGGER: str.append("ER_BACKUP_CATALOG_ADD_TRIGGER");
break;
+ case BSTREAM_IT_PRIVILEGE: str.append("ER_BACKUP_CATALOG_ADD_PRIV");
break;
+ default: break;
+ }
+ if (str.ptr())
+ {
+ DBUG_EXECUTE_IF(str.ptr(),
+ m_log.report_error(error, db.name().ptr(), obj->get_name()->ptr());
+ return NULL;);
+ }
+ // Tablespace is a special case since Falcon is disabled.
+ DBUG_EXECUTE_IF("ER_BACKUP_CATALOG_ADD_TS",
+ m_log.report_error(ER_BACKUP_CATALOG_ADD_TS,
+ "Debug insertion test");
+ return NULL;);
+
5. Have you considered moving this error injection code to a separate
method? The same applies to similar error injection in kernel.cc
--
Jørgen Løland
