MySQL Bugs: #43603: mysqltest command disable_abort_on_error does not affect all built-in commands

Bug #43603	mysqltest command disable_abort_on_error does not affect all built-in commands
Submitted:	12 Mar 2009 16:16	Modified:	14 Oct 2010 13:29
Reporter:	Ingo Strüwing	Email Updates:
Status:	Closed	Impact on me:	None
Category:	Tools: MTR / mysql-test-run	Severity:	S3 (Non-critical)
Version:	6.0	OS:	Any
Assigned to:	Bjørn Munch	CPU Architecture:	Any

Description:
The mysqltest command 'disable_abort_on_error' does not affect [all?] built-in commands.

I tested it with 'exec', 'remove_file', and 'copy_file'. I guess that other built-in commands are also affected. Perhaps even all of them.

I think this is a problem, because these commands can rely on the results of SQL statements. If 'disable_abort_on_error' continues after an unexpected failure of these, the following built-in command may fail as a follow-up error. For example a BACKUP statement creates a backup image file, if it succeeds. If I try to ignore an error of this statement, the test script may proceed to the 'remove_file' command, which tries to clean up with the backup image file. But if BACKUP failed, the file won't exist and 'remove_file' will fail. I would like, the 'disable_abort_on_error' command would also affect the 'remove_file' command so that the test case can continue. The workaround to preceed 'remove_file' with 'error 0,1' is not optimal as it won't check the file existence any more. That is, it won't detect if a successful BACKUP does not produce a file with the expected [path and] name. That is, when I remove the 'disable_abort_on_error' command later.

BTW, I am unsure if 'file_exists' should work normally under 'disable_abort_on_error'. That is, if it should stop a test case. Maybe it would be better, not to break, but report a failure instead.

How to repeat:
Select an arbitrary test file, e.g. backup_default.test
Insert the following sequence, e.g. at the end:

--disable_abort_on_error
--exec illegal_sh_command
--remove_file non_existent_file
--copy_file non_existent_file /non_existent_directory
ILLEGAL_SQL_STATEMENT;
SHOW PROCESSLIST;
--enable_abort_on_error

If you comment out one of the built-in commands after the other, you will see that it fails on the next one respectively. If you comment out all built-in commands, you will see that 'disable_abort_on_error' does affect the SQL statement 'ILLEGAL_SQL_STATEMENT' and the test proceeds to 'SHOW PROCESSLIST'.

Suggested fix:
Check the "abort on error flag" in every built-in command.

Suggest triage values:
Defect: medium. I'm unsure if this might be as designed. :(
Workaround: unacceptable. Using error 0,1 has disadvantages.
Impact: minimal. Feature used during test case development [almost] only.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/103662

2903 Bjorn Munch	2010-03-18
      Bug #43603 mysqltest command disable_abort_on_error does not affect all built-in commands
      After disable_abort_on_error, behaved as if --error was in effect
      Add condition before die, as after queries

With disable_abort_on_error, non-query commands that failed would be reported as "failed with wrong error", as if it didn't match error codes given in --error.

There is faulty logic here, the die() should be on condition that we're not expecting any particular error, like it does for queries. So I added an 

    if (command->expected_errors.count > 0)

Also, cat_file was changed to not fail unconditionally but use the same system.

It could be interesting to know why cat_file failed. With patch, the cause is lost(not that it was very apparent before either). Maybe it should print error before returning 1?
Looks ok.

I just figured cat_file should behave the same way as e.g. remove_file, found the problem when I used cat_file in the mysqltest.test test case.

The fact that the *_file commands don't print more info should IMHO be regarded as a separate issue.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/103952

2905 Bjorn Munch	2010-03-22
      Bug #43603 mysqltest command disable_abort_on_error does not affect all built-in commands
      After disable_abort_on_error, behaved as if --error was in effect
      Add condition before die, as after queries

Pushed to *-mtr branches

Pushed into mysql-trunk 5.6.1-m4 (revid:alik@sun.com-20100816062701-qo9dpnk5tkt1pksb) (version source revid:alik@sun.com-20100816062603-xc16eftmv7rmktyq) (merge vers: 5.6.1-m4) (pib:20)

Pushed into mysql-next-mr (revid:alik@sun.com-20100816062819-bluwgdq8q4xysmlg) (version source revid:alik@sun.com-20100816062612-enatdwnv809iw3s9) (pib:20)

Test suite changes. No changelog entry needed.

Pushed into mysql-5.1 5.1.52 (revid:sunanda.menon@sun.com-20100928083322-wangbv97uobu7g66) (version source revid:sunanda.menon@sun.com-20100928083322-wangbv97uobu7g66) (merge vers: 5.1.52) (pib:21)

Pushed into mysql-5.1-telco-7.0 5.1.51-ndb-7.0.20 (revid:martin.skold@mysql.com-20101014082627-jrmy9xbfbtrebw3c) (version source revid:martin.skold@mysql.com-20101014082627-jrmy9xbfbtrebw3c) (merge vers: 5.1.51-ndb-7.0.20) (pib:21)

Pushed into mysql-5.1-telco-6.3 5.1.51-ndb-6.3.39 (revid:martin.skold@mysql.com-20101014083757-5qo48b86d69zjvzj) (version source revid:martin.skold@mysql.com-20101014083757-5qo48b86d69zjvzj) (merge vers: 5.1.51-ndb-6.3.39) (pib:21)

Pushed into mysql-5.1-telco-6.2 5.1.51-ndb-6.2.19 (revid:martin.skold@mysql.com-20101014084420-y54ecj85j5we27oa) (version source revid:martin.skold@mysql.com-20101014084420-y54ecj85j5we27oa) (merge vers: 5.1.51-ndb-6.2.19) (pib:21)

Set back to Closed state. See previous comments.