Merge lp:~gl-az/percona-xtrabackup/2.0-testrun-summary into lp:percona-xtrabackup/2.0

[George Ormond Lorch III]

Added test status tracking and summary report at end of test run cycle to make it wasy to see what failed at a glance, similar to MTR.

[Laurynas Biveinis (laurynas-biveinis)]

Doesn't this report the skipped testcases (exit code == 200) as the failed ones?

If yes, that wouldn't be entirely correct, I'd report failed and skipped testcases separately.

Typo on diff line 32.

review: Needs Fixing

[George Ormond Lorch III (gl-az)]

Good catch Laurynas, I didn't think about the skipped cases in this context. Something else worth considering is making the testrun behave more like the suite and MTR in that it will stop on the first failure and return that error code, which can be overridden by a -f and maybe add some retry logic.

[Laurynas Biveinis (laurynas-biveinis)]

It looks that now the skipped cases will be listed twice: both as skipped (exit == 200), and as failed (exit != 0).

review: Needs Fixing

[George Ormond Lorch III (gl-az)]

Oy, Doh! That's what I get for trying to do n+1 things at once where 'n'
is my max capacity for parallelism...

On 6/3/2013 9:34 PM, Laurynas Biveinis wrote:
> Review: Needs Fixing
>
> It looks that now the skipped cases will be listed twice: both as skipped (exit == 200), and as failed (exit != 0).
>

--
George O. Lorch III
Software Engineer, Percona
[phone] x542 US/Arizona (GMT -7)
skype: george.ormond.lorch.iii

[George Ormond Lorch III (gl-az)]

OK Laurynas, as we discussed on IRC I did some more testing:
Created a new 'suite' called 'u' which exists as a sibling directory to the current 't' (test/u).
Added 3 simple tests:

$ cat u/test_pass.sh
. inc/common.sh
sleep 5
exit 0

$ cat u/test_fail.sh
. inc/common.sh
sleep 5
exit 1

$ cat u/test_skip.sh
. inc/common.sh
sleep 5
exit $SKIPPED_EXIT_CODE

Then ran the suite:
$ ./testrun -j 1 -s u

The first thing I ran into was all tests were failing with 255, why? Well, testrun.c doesn't work with alternate suites correctly. It will find the tests in the correct location, but when it goes to execute them, it tries to execute them out of ./t/<testname> instead of ./<suitename>/<testname>. This was a simple fix so I made it and updated the branch with this fix.

With that working, I run again and realize that testrun.c launches all tests individually through testrun.sh. testrun.sh only returns success (0) or failure (-1/255), nothing else because it may have run multiple tests so it only returns 'all succeeded or skipped' or 'something failed'. With this, in order to report skipped tests, we will need to modify testrun.sh to do something like pass the result from the tests directly out of there was only one test specified, otherwise, use the old method.

So, that being said, we have two choices here:
1) We just go back to my original work where we were really only interested in reporting true failures and accepting skipped tests as passed.
2) Change testrun.sh to return the true result when only one test was specified.

Opinions?

[lp:~gl-az/percona-xtrabackup/2.0-testrun-summary]

561. By George Ormond Lorch III on 2013-06-05

Fixed issue in testrun.c where specifying testsuite would find the tests in the corredt directory but would still try to execute them out of ./t

Added test status tracking and summary report at end of test run cycle to make it wasy to see what passed, skipped and failed at a glance, similar to MTR.

Changed testrun.sh slightly to return same code out of testrun.sh as the test that was executed when only a single test was run.

Unmerged revisions

561. By George Ormond Lorch III on 2013-06-05

Fixed issue in testrun.c where specifying testsuite would find the tests in the corredt directory but would still try to execute them out of ./t

Added test status tracking and summary report at end of test run cycle to make it wasy to see what passed, skipped and failed at a glance, similar to MTR.

Changed testrun.sh slightly to return same code out of testrun.sh as the test that was executed when only a single test was run.

=== modified file 'test/testrun.c'
--- test/testrun.c	2012-11-30 04:28:06 +0000
+++ test/testrun.c	2013-06-04 17:49:27 +0000
@@ -41,6 +41,7 @@
   char *buf;
   size_t end_of_buf;
   size_t bufsz;
+  int status;
 };
 
 pid_t *childpid;
@@ -96,6 +97,7 @@
     (*cases)[i].buf= NULL;
     (*cases)[i].end_of_buf= 0;
     (*cases)[i].bufsz= 0;
+    (*cases)[i].status= 0;
     free(namelist[i]);
   }
 
@@ -113,6 +115,38 @@
   free(cases);
 }
 
+static void report_status(struct testcase *cases, int n)
+{
+  int i;
+  int failed=0;
+  int skipped=0;
+  for (i=0; i<n; i++)
+    if (WEXITSTATUS(cases[i].status) != 0)
+      if (WEXITSTATUS(cases[i].status) == 200)
+        skipped++;
+      else
+        failed++;
+
+  printf("SUMMARY: %d run, %d successful, %d skipped, %d failed\n", n, n - failed - skipped, skipped, failed);
+  if (skipped)
+  {
+    printf("Skipped cases:");
+    for (i=0; i<n; i++) 
+      if (WEXITSTATUS(cases[i].status) == 200)
+        printf(" %s", cases[i].name);
+    printf("\n");
+  }
+
+  if (failed)
+  {
+    printf("Failing cases:");
+    for (i=0; i<n; i++) 
+      if (WEXITSTATUS(cases[i].status) != 0 && WEXITSTATUS(cases[i].status != 200))
+        printf(" %s", cases[i].name);
+    printf("\n");
+  }
+}
+
 static int run_testcase_in_child(int nr, struct testcase *t, pid_t *cpid, const char* xbtarget)
 {
   int fd[2];
@@ -202,7 +236,6 @@
   int nfds= 0;
   int retval;
   pid_t chpid[njobs];
-  int status;
   int next_testcase= 0;
   int i;
   fd_set rfds;
@@ -277,8 +310,8 @@
 	    t->end_of_buf+= r;
 	} while(r>0);
 
-	pid_t waited= waitpid(childpid[i], &status, WNOHANG);
-	if (!(WIFEXITED(status) || WIFSIGNALED(status)))
+	pid_t waited= waitpid(childpid[i], &testcases[childtest[i]].status, WNOHANG);
+	if (!(WIFEXITED(testcases[childtest[i]].status) || WIFSIGNALED(testcases[childtest[i]].status)))
 	  continue;
 
 	if (waited != childpid[i])
@@ -289,7 +322,7 @@
 
 	close(childfd[i]);
 	printf("[%d] completed %s status %d\n",
-	       i, testcases[childtest[i]].name, WEXITSTATUS(status));
+	       i, testcases[childtest[i]].name, WEXITSTATUS(testcases[childtest[i]].status));
 	childfd[i]=-1;
 	nchildren--;
 
@@ -399,6 +432,8 @@
 
   run_testcases(testcases, nrcases, njobs, timeout, xbtarget);
 
+  report_status(testcases, nrcases);
+
   free_testcases(testcases, nrcases);
 
   return 0;