I've had a look at this myself. There is no way to reproduce this error at a
lower level without additional heroics. The error from open_tables() is
beyond the scope of backup. We are not testing what happens when
open_tables() fails, we are testing how backup would handle such a failure.
The real issue is that the original code never returned an error status. We
are asking, "what happens if this method returns true?" In that case, debug
insertion is perfectly valid.
> Fix look good, but I am not sure I like the mechanism used in
> the second test case:
>
> > + DBUG_EXECUTE_IF("backup_fail_add_trigger", DBUG_RETURN(TRUE););
> > if (open_tables(thd, &lst, &num_tables, 0))
> > - DBUG_RETURN(FALSE);
> > + DBUG_RETURN(TRUE);
> >
> > DBUG_ASSERT(num_tables == 1);
> > Table_triggers_list *triggers= lst->table->triggers;
>
> Any particular reason we should test that the code works if
> TRUE is returned from this particular point in this function,
> and not from all the other places that may return TRUE? I do
> not think it makes sense to add such "error triggers" for all
> potential errors in the code, so what makes this code exceptional?