MySQL Bugs: #38833: mysql-test-run needs diff. Problem w/ error handling in calling diff on Windows

Bug #38833	mysql-test-run needs diff. Problem w/ error handling in calling diff on Windows
Submitted:	16 Aug 2008 7:22	Modified:	21 Jan 2009 16:17
Reporter:	Daniel Fischer	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server: Tests	Severity:	S3 (Non-critical)
Version:	5.1+	OS:	Windows (WS2008)
Assigned to:	Patrick Crews	CPU Architecture:	Any

Description:
17 funcs1 test cases fail on Windows because they assume a UNIX environment:

funcs_1.innodb_storedproc [ fail ]

'"diff"' is not recognized as an internal or external command,
operable program or batch file.

funcs_1.innodb_trig_03 [ fail ]

'"diff"' is not recognized as an internal or external command,
operable program or batch file.

funcs_1.innodb_trig_03e [ fail ]

'"diff"' is not recognized as an internal or external command,
operable program or batch file.

funcs_1.innodb_trig_0407 [ fail ]

'"diff"' is not recognized as an internal or external command,
operable program or batch file.

funcs_1.innodb_trig_08 [ fail ]

'"diff"' is not recognized as an internal or external command,
operable program or batch file.

funcs_1.is_tables [ fail ]

'"diff"' is not recognized as an internal or external command,
operable program or batch file.

funcs_1.memory_storedproc [ fail ]

'"diff"' is not recognized as an internal or external command,
operable program or batch file.

funcs_1.memory_trig_03 [ fail ]

'"diff"' is not recognized as an internal or external command,
operable program or batch file.

funcs_1.memory_trig_03e [ fail ]

'"diff"' is not recognized as an internal or external command,
operable program or batch file.

funcs_1.memory_trig_0407 [ fail ]

'"diff"' is not recognized as an internal or external command,
operable program or batch file.

funcs_1.memory_trig_08 [ fail ]

'"diff"' is not recognized as an internal or external command,
operable program or batch file.

funcs_1.myisam_storedproc [ fail ]

'"diff"' is not recognized as an internal or external command,
operable program or batch file.

funcs_1.myisam_trig_03 [ fail ]

'"diff"' is not recognized as an internal or external command,
operable program or batch file.

funcs_1.myisam_trig_03e [ fail ]

'"diff"' is not recognized as an internal or external command,
operable program or batch file.

funcs_1.myisam_trig_0407 [ fail ]

'"diff"' is not recognized as an internal or external command,
operable program or batch file.

funcs_1.myisam_trig_08 [ fail ]

'"diff"' is not recognized as an internal or external command,
operable program or batch file.

funcs_1.processlist_val_no_prot [ fail ]

'"diff"' is not recognized as an internal or external command,
operable program or batch file.

How to repeat:
Run the test suite on Windows without Cygwin.

Suggested fix:
Implement a diff command in mysqltest. It seems to be required by many test cases, also in other suites.

problem would also be seen on netware..

These tests aren't using diff.  What is happening is that a test failure results in a call to diff for displaying the differences between <test>.result and <test>.reject.

MTR needs to be updated to call fc or comp on Windows machines that don't have a version of diff available for use.

This might be specific to diffing files in 'suite' as opposed to reject/result in the main suite

(Renamed the test to better reflect the problem)

Good then we are in agreement whet the problem is. Lars, can you find someone to fix it please?

mysqltest.c's run_command function gets a return value of '1' regardless of whether diff is available or not on Windows.

I suspect that we'll need to maybe do some output verification to solve this problem

.test file for a quickly failing test case to see the error on Windows.

Attachment: helper.test (, text), 12 bytes.

.result file for a quickly failing test case to see the bug.

Attachment: helper.result (, text), 16 bytes.

$ bzr dif mysqltest.c
=== modified file 'client/mysqltest.c'
--- client/mysqltest.c 2008-09-11 16:40:15 +0000
+++ client/mysqltest.c 2008-10-08 20:00:38 +0000
@@ -1457,6 +1457,12 @@
   if (init_dynamic_string(&ds_tmp, "", 256, 256))
     die("Out of memory");

+  /*
+    Most "diff" tools return >1 if error
+    But "no such command" on windows return 1 as well
+  */
+  int ok= IF_WIN(0, 1);
+
   /* First try with unified diff */
   if (run_tool("diff",
                &ds_tmp, /* Get output from diff in ds_tmp */
@@ -1464,7 +1470,7 @@
                filename1,
                filename2,
                "2>&1",
-               NULL) > 1) /* Most "diff" tools return >1 if error */
+               NULL) > ok) /* Most "diff" tools return >1 if error */
   {
     dynstr_set(&ds_tmp, "");

@@ -1475,7 +1481,7 @@
                  filename1,
                  filename2,
                  "2>&1",
-                 NULL) > 1) /* Most "diff" tools return >1 if error */
+                 NULL) > ok)
     {
       dynstr_set(&ds_tmp, "");

@@ -1485,7 +1491,7 @@
                    filename1,
                    filename2,
                    "2>&1",
-                   NULL) > 1) /* Most "diff" tools return >1 if error */
+                   NULL) > ok) /* Most "diff" tools return >1 if error */
       {
         dynstr_set(&ds_tmp, "");

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/61562

2749 Patrick Crews	2008-12-12
      Bug#38833: mysql-test-run needs diff.  Problem w/ error handling in calling diff on Windows
      
      Altered expected return value for diff calls on Windows.
      
      We see 'diff' return a value of 1 whether it is successful or not found.

OK, but it seemss to me that on Windows, it won't be able to distinguish between a diff finding a diff vs. not being able to run diff?

Also, perhaps the comment after ... > ok) should be rephrased a bit?

That is right, in 5.1 it will first run "diff --help" to check if diff exists at all. Maybe that should be backported to 5.0?

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/61833

2746 Patrick Crews	2008-12-16
      Bug#38833:  mysql-test-run needs diff.  Problem w/error handling in calling diff on Windows
      
      Altered mysqltest.c's run_command function to catch a lack of diff on Windows
      so that a user receives a proper error message.

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/61914

2746 Patrick Crews	2008-12-17
      Bug#38833: mysql-test-run needs diff.
      
      Added code to detect if diff isn't available on Windows.
      diff returns 1 on that OS whether it is successful, fails, or is not present.
      
      Have to detect error pattern in diff output to properly inform user they need the utility.

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/61966

2747 Patrick Crews	2008-12-18
      Bug#38833: mysql-test-run needs diff.  Problem w/ error handling in calling diff on Windows
      
      Altered run_command to look for an output pattern that indicates diff is missing.
      This is needed since diff returns 1 on Windows whether the utility is successful or
      not present.

I'm OK with this fix

I'd rather see that the logic was more "held together", like a function "check_for_diff"  that returns true or false

And then
if (!check_for_diff())
  print error message about downloading diff.

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/62019

2746 Patrick Crews	2008-12-18
      Bug#38833: mysql-test-run needs diff.  Problem w/ error handling in calling diff on Windows.
      
      Altered mysqltest.c to include a test for diff that will work on Windows.

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/62102

2747 Patrick Crews	2008-12-19
      Bug#38833:  mysql-test-run needs diff.  Problem w/ error handling in calling diff on Windows.
      
      Removed unused variable and added a comment to explain how we are testing for diff.

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/62109

2746 Patrick Crews	2008-12-19
      Bug#38833: mysql-test-run needs diff.  Problem w/ error handling in calling diff on Windows.
      
      Added a check for diff and altered show_diff accordingly.
      We look to see if diff -v is productive and display an informative error message if
      diff is not available.

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/62866

2726 Patrick Crews	2009-01-09
      Bug#38833: mysql-test-run neeeds diff. Problem w/ error handling in calling diff on Windows.
      
      Changed variable cmd in function have_diff to fix compiler warnings on Pushbuild (sapsvr)

Pushed into 5.1.31 (revid:joro@sun.com-20090115053147-tx1oapthnzgvs1ro) (version source revid:davi.arnaut@sun.com-20090113150631-y84w12o2zmh7j3qd) (merge vers: 5.1.31) (pib:6)

Pushed into 5.0.77 (revid:joro@sun.com-20090115043518-cn6jeeg78j6mupp3) (version source revid:azundris@mysql.com-20090109154903-4obc0bufq87udki4) (merge vers: 5.0.76) (pib:6)

Test suite changes. No changelog entry needed.

Setting report to NDI pending push into 6.0.x.

Pushed into 5.1.31-ndb-6.2.17 (revid:tomas.ulin@sun.com-20090119095303-uwwvxiibtr38djii) (version source revid:tomas.ulin@sun.com-20090115073240-1wanl85vlvw2she1) (merge vers: 5.1.31-ndb-6.2.17) (pib:6)

Pushed into 5.1.31-ndb-6.3.21 (revid:tomas.ulin@sun.com-20090119104956-guxz190n2kh31fxl) (version source revid:tomas.ulin@sun.com-20090119104956-guxz190n2kh31fxl) (merge vers: 5.1.31-ndb-6.3.21) (pib:6)

Setting status back to NDI pending merge to 6.0 tree.

Pushed into 5.1.31-ndb-6.4.1 (revid:tomas.ulin@sun.com-20090119144033-4aylstx5czzz88i5) (version source revid:tomas.ulin@sun.com-20090119144033-4aylstx5czzz88i5) (merge vers: 5.1.31-ndb-6.4.1) (pib:6)

Pushed into 6.0.10-alpha (revid:joro@sun.com-20090119171328-2hemf2ndc1dxl0et) (version source revid:patrick.crews@sun.com-20090107205339-uu1tgjsogjnq2w20) (merge vers: 6.0.10-alpha) (pib:6)

Test suite changes. No changelog entry needed.