Mailing-List: contact commits-help@hawq.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@hawq.incubator.apache.org
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: odiachenko@apache.org
To: commits@hawq.incubator.apache.org
Message-Id: <9587126ee40c4a8c9672efe4c8732f73@git.apache.org>
Subject: incubator-hawq git commit: HAWQ-400. Support expected exit codes for
 regression tests. [Forced Update!]
Date: Wed, 17 Feb 2016 00:47:18 +0000 (UTC)

Repository: incubator-hawq
Updated Branches:
  refs/heads/HAWQ-400 4c7469171 -> c74ef60b2 (forced update)


HAWQ-400. Support expected exit codes for regression tests.


Project: http://git-wip-us.apache.org/repos/asf/incubator-hawq/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-hawq/commit/c74ef60b
Tree: http://git-wip-us.apache.org/repos/asf/incubator-hawq/tree/c74ef60b
Diff: http://git-wip-us.apache.org/repos/asf/incubator-hawq/diff/c74ef60b

Branch: refs/heads/HAWQ-400
Commit: c74ef60b2adf0b73ce7ed365d2ac738111ee330d
Parents: 37a5043
Author: Oleksandr Diachenko <odiachenko@pivotal.io>
Authored: Fri Feb 12 21:07:33 2016 -0800
Committer: Oleksandr Diachenko <odiachenko@pivotal.io>
Committed: Tue Feb 16 16:47:15 2016 -0800

----------------------------------------------------------------------
 src/test/regress/GNUmakefile       |   2 +-
 src/test/regress/README            |  18 ++
 src/test/regress/expected_statuses |   1 +
 src/test/regress/pg_regress.c      | 295 ++++++++++++++++++++++----------
 4 files changed, 224 insertions(+), 92 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/c74ef60b/src/test/regress/GNUmakefile
----------------------------------------------------------------------
diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile
index 7b3f51d..841d163 100644
--- a/src/test/regress/GNUmakefile
+++ b/src/test/regress/GNUmakefile
@@ -143,7 +143,7 @@ installcheck-parallel: all upg2-setup ugpart-setup
 	$(pg_regress_call)  --psqldir=$(PSQLDIR) --schedule=$(srcdir)/parallel_schedule --srcdir=$(abs_srcdir)
 
 installcheck-good: all ./current_good_schedule upg2-setup ugpart-setup
-	$(pg_regress_call)  --psqldir=$(PSQLDIR) --schedule=./current_good_schedule --srcdir=$(abs_srcdir)
+	$(pg_regress_call)  --psqldir=$(PSQLDIR) --schedule=./current_good_schedule --srcdir=$(abs_srcdir) --expected-statuses-file=expected_statuses
 
 installcheck-goh: all ./goh_schedule upg2-setup ugpart-setup
 	$(pg_regress_call)  --psqldir=$(PSQLDIR) --schedule=./goh_schedule --srcdir=$(abs_srcdir) --tablespace=hdfs_ts

http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/c74ef60b/src/test/regress/README
----------------------------------------------------------------------
diff --git a/src/test/regress/README b/src/test/regress/README
index 821905d..50e7013 100644
--- a/src/test/regress/README
+++ b/src/test/regress/README
@@ -290,3 +290,21 @@ float8/i.86-.*-openbsd=float8-small-is-zero
    variant that seems to work best. Therefore it is safest to use this
    mechanism only for variant results that you are willing to consider
    equally valid in all contexts.
+
+                          Negative status codes
+   Most of regression tests are expected to exit with code 0, but regression test
+   framework also supports non-0 exit codes. All non-0 expected codes should be included in
+   expected statuses file, which is "src/test/regress/expected_status" by default or could be
+   overridden by cli argument "expected-statuses-file".
+
+   For example:
+
+   ./pg_regress --expected-statuses-file=/tmp/expected_statuses
+
+   Format of expected statuses file:
+
+   test_name:exit_code
+
+   Max line length is 1024 characters.
+   Test will be marked as failed if actual and expected exit codes doesn't correspond even if output
+   was expected.

http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/c74ef60b/src/test/regress/expected_statuses
----------------------------------------------------------------------
diff --git a/src/test/regress/expected_statuses b/src/test/regress/expected_statuses
new file mode 100644
index 0000000..16c50d1
--- /dev/null
+++ b/src/test/regress/expected_statuses
@@ -0,0 +1 @@
+hcatalog_lookup:2
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/incubator-hawq/blob/c74ef60b/src/test/regress/pg_regress.c
----------------------------------------------------------------------
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index b222a0b..ede0e2b 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -41,6 +41,14 @@ typedef struct _resultmap
 	struct _resultmap *next;
 }	_resultmap;
 
+/*linked list of statuses for tests*/
+typedef struct _statuslist
+{
+	char	*test;
+	int		status;
+	struct	_statuslist *next;
+} _statuslist;
+
 /*
  * Values obtained from pg_config_paths.h and Makefile.
  * In non-temp_install mode, the only thing we need is the location of psql,
@@ -84,15 +92,15 @@ static char *user = NULL;
 static char *srcdir = NULL;
 static _stringlist *extraroles = NULL;
 static char *initfile = "./init_file";
-static char *tablespace = "";
+static char *expected_statuses_file = "expected_statuses";
 
 /* internal variables */
 static const char *progname;
 static char *logfilename;
 static FILE *logfile;
 static char *difffilename;
-
 static _resultmap *resultmap = NULL;
+static _statuslist *expected_statuses = NULL;
 
 static int	success_count = 0;
 static int	fail_count = 0;
@@ -122,6 +130,11 @@ psql_command(const char *database, const char *query,...)
 /* This extension allows gcc to check the format string for consistency with
    the supplied arguments. */
 __attribute__((format(printf, 2, 3)));
+static void
+log_child_exit(int exitstatus, int expected_status);
+
+static bool
+is_status_expected(int exit_status, int expected_status);
 
 #ifdef WIN32
 typedef BOOL (WINAPI * __CreateRestrictedToken) (HANDLE, DWORD, DWORD, PSID_AND_ATTRIBUTES, DWORD, PLUID_AND_ATTRIBUTES, DWORD, PSID_AND_ATTRIBUTES, PHANDLE);
@@ -193,6 +206,21 @@ free_stringlist(_stringlist ** listhead)
 }
 
 /*
+ * Free a statuslist.
+ */
+static void
+free_statuslist(_statuslist ** listhead)
+{
+	if (listhead == NULL || *listhead == NULL)
+		return;
+	if ((*listhead)->next != NULL)
+		free_statuslist(&((*listhead)->next));
+	free((*listhead)->test);
+	free(*listhead);
+	*listhead = NULL;
+}
+
+/*
  * Split a delimited string into a stringlist
  */
 static void
@@ -529,6 +557,78 @@ convert_sourcefiles(void)
 }
 
 /*
+ * Load expected statuses from given file name.
+ * Line format is
+ * 	test_name:expected_return_code
+ */
+static void
+load_expected_statuses(char *filename)
+{
+	char		buf[MAXPGPATH];
+	FILE		*f;
+	int			i;
+	int 		status;
+	int			sep_index;
+	char		*sep_ptr;
+	bool		line_valid;
+
+	f = fopen(filename, "r");
+	if (!f)
+	{
+		fprintf(stderr, _("could not open file with expected statuses for reading: \n"));
+		exit_nicely(2);
+	}
+
+	while (fgets(buf, sizeof(buf), f))
+	{
+
+		i = strlen(buf);
+		while (i > 0 && isspace((unsigned char) buf[i - 1]))
+			buf[--i] = '\0';
+
+		sep_ptr = strchr(buf, ':');
+
+		if (sep_ptr == NULL)
+		{
+			fprintf(stderr, _("incorrect expected statuses entry: %s\n"), buf);
+			exit_nicely(2);
+		}
+
+		sep_index = sep_ptr - buf;
+
+		char test[sep_index+1];
+		memset(test, 0, sizeof(test));
+
+		strncpy(test, buf, sep_index);
+		test[sep_index+1] = '\0';
+
+
+		char status_str[strlen(buf)-sep_index];
+		memset(status_str, 0, sizeof(status_str));
+		strncpy(status_str, buf + sep_index +1, strlen(buf));
+		status_str[sep_index+1] = '\0';
+
+		line_valid = (sscanf(status_str, "%d", (int *) (&status)) == 1);
+
+		if (!line_valid)
+		{
+			fprintf(stderr, _("incorrect expected statuses entry: %s\n"), buf);
+			exit_nicely(2);
+		}
+
+		_statuslist *entry = malloc(sizeof(_statuslist));
+
+		entry->test = strdup(test);
+		entry->status = status;
+		entry->next = expected_statuses;
+		expected_statuses = entry;
+
+	}
+
+	fclose(f);
+}
+
+/*
  * Scan resultmap file to find which platform-specific expected files to use.
  *
  * The format of each line of the file is
@@ -620,6 +720,27 @@ load_resultmap(void)
 }
 
 /*
+ * Get expected status for given test
+ */
+static
+const int
+get_expected_status(const char *test)
+{
+	_statuslist		*es;
+
+	for (es = expected_statuses; es != NULL; es = es->next)
+	{
+		if (strcmp(test, es->test) == 0)
+		{
+			return es->status;
+		}
+	}
+
+	return 0; //default value
+}
+
+
+/*
  * Check in resultmap if we should be looking at a different file
  */
 static
@@ -770,6 +891,7 @@ initialize_environment(void)
 
 	convert_sourcefiles();
 	load_resultmap();
+	load_expected_statuses(expected_statuses_file);
 }
 
 /*
@@ -1333,32 +1455,70 @@ wait_for_tests(PID_TYPE * pids, int *statuses, char **names, int num_tests)
 }
 
 /*
- * report nonzero exit code from a test process
+ * Print test status depending on differences, actual, expected statuses
  */
 static void
-log_child_failure(int exitstatus)
+print_test_status(bool differ, int actual_status, int expected_status, double diff_secs)
 {
-	if (WIFEXITED(exitstatus))
-		status(_(" (test process exited with exit code %d)"),
-			   WEXITSTATUS(exitstatus));
-	else if (WIFSIGNALED(exitstatus))
-	{
-#if defined(WIN32)
-		status(_(" (test process was terminated by exception 0x%X)"),
-			   WTERMSIG(exitstatus));
-#elif defined(HAVE_DECL_SYS_SIGLIST) && HAVE_DECL_SYS_SIGLIST
-		status(_(" (test process was terminated by signal %d: %s)"),
-			   WTERMSIG(exitstatus),
-			   WTERMSIG(exitstatus) < NSIG ?
-			   sys_siglist[WTERMSIG(exitstatus)] : "(unknown))");
-#else
-		status(_(" (test process was terminated by signal %d)"),
-			   WTERMSIG(exitstatus));
-#endif
+
+	if (!differ && is_status_expected(actual_status, expected_status))
+	{
+		status(_("ok"));
+		if (diff_secs > 0)
+			status(_(" (%.2f sec)"), diff_secs);
+		success_count++;
+	} else
+	{
+		status(_("FAILED"));
+		if (diff_secs > 0)
+			status(_(" (%.2f sec)"), diff_secs);
+		fail_count++;
+		log_child_exit(actual_status, expected_status);
+	}
+
+	status_end();
+}
+
+/*
+ * Returns true if actual exit code was expected
+ */
+static bool
+is_status_expected(int exit_status, int expected_status)
+{
+	return (WEXITSTATUS(exit_status) == expected_status);
+}
+
+
+/*
+ * Report unexpected exit code or termination by signal for test process
+ */
+static void
+log_child_exit(int exitstatus, int expected_status)
+{
+	if (!is_status_expected(exitstatus, expected_status))
+	{
+		if (WIFEXITED(exitstatus))
+			status(_(" (test process exited with unexpected exit code %d, but was expected exit code %d)"),
+				WEXITSTATUS(exitstatus), expected_status);
+		else if (WIFSIGNALED(exitstatus))
+		{
+	#if defined(WIN32)
+			status(_(" (test process was terminated by exception 0x%X)"),
+				   WTERMSIG(exitstatus));
+	#elif defined(HAVE_DECL_SYS_SIGLIST) && HAVE_DECL_SYS_SIGLIST
+			status(_(" (test process was terminated by signal %d: %s)"),
+				   WTERMSIG(exitstatus),
+				   WTERMSIG(exitstatus) < NSIG ?
+				   sys_siglist[WTERMSIG(exitstatus)] : "(unknown))");
+	#else
+			status(_(" (test process was terminated by signal %d)"),
+				   WTERMSIG(exitstatus));
+	#endif
+		}
+		else
+			status(_(" (test process exited with unrecognized status %d)"),
+				   exitstatus);
 	}
-	else
-		status(_(" (test process exited with unrecognized status %d)"),
-			   exitstatus);
 }
 
 /*
@@ -1374,7 +1534,6 @@ run_schedule(const char *schedule, test_function tfunc)
 	_stringlist *tags[MAX_PARALLEL_TESTS];
 	PID_TYPE	pids[MAX_PARALLEL_TESTS];
 	int			statuses[MAX_PARALLEL_TESTS];
-	_stringlist *ignorelist = NULL;
 	char		scbuf[1024];
 	FILE	   *scf;
 	int			line_num = 0;
@@ -1401,6 +1560,8 @@ run_schedule(const char *schedule, test_function tfunc)
 		struct timeval start_time, end_time;
 		double		diff_secs;
 
+
+
 		line_num++;
 
 		for (i = 0; i < MAX_PARALLEL_TESTS; i++)
@@ -1425,6 +1586,7 @@ run_schedule(const char *schedule, test_function tfunc)
 
 			test = scbuf + 6;
 
+
 			/* MPP-9643: allow ability to disable tests per platform */
 			snprintf(cmd, sizeof(cmd),
 					 SYSTEMQUOTE "gpexclude.pl --test %s --exclude %s.EXCLUDE  --quiet  " SYSTEMQUOTE, test, schedule);
@@ -1434,8 +1596,6 @@ run_schedule(const char *schedule, test_function tfunc)
 				c = test;
 				while (*c && isspace((unsigned char) *c))
 						c++;
-				add_stringlist_item(&ignorelist, c);
-
 				/*
 				 * Note: ignore: lines do not run the test, they just
 				 * say that failure of this test when run later on is
@@ -1451,7 +1611,6 @@ run_schedule(const char *schedule, test_function tfunc)
 			c = scbuf + 8;
 			while (*c && isspace((unsigned char) *c))
 				c++;
-			add_stringlist_item(&ignorelist, c);
 
 			/*
 			 * Note: ignore: lines do not run the test, they just say that
@@ -1471,6 +1630,8 @@ run_schedule(const char *schedule, test_function tfunc)
 		inword = false;
 		for (c = test; *c; c++)
 		{
+
+
 			if (isspace((unsigned char) *c))
 			{
 				*c = '\0';
@@ -1585,42 +1746,9 @@ run_schedule(const char *schedule, test_function tfunc)
 				differ |= newdiff;
 			}
 
-			if (differ)
-			{
-				bool		ignore = false;
-				_stringlist *sl;
-
-				for (sl = ignorelist; sl != NULL; sl = sl->next)
-				{
-					if (strcmp(tests[i], sl->str) == 0)
-					{
-						ignore = true;
-						break;
-					}
-				}
-				if (ignore)
-				{
-					status(_("failed (ignored)"));
-					fail_ignore_count++;
-				}
-				else
-				{
-					status(_("FAILED"));
-    				status(_(" (%.2f sec)"), diff_secs);
-					fail_count++;
-				}
-			}
-			else
-			{
-				status(_("ok"));
-				status(_(" (%.2f sec)"), diff_secs);
-				success_count++;
-			}
-
-			if (statuses[i] != 0)
-				log_child_failure(statuses[i]);
+			int expected_status = get_expected_status(test);
 
-			status_end();
+			print_test_status(differ, statuses[i], expected_status, -1);
 		}
 	}
 
@@ -1672,19 +1800,9 @@ run_single_test(const char *test, test_function tfunc)
 		differ |= newdiff;
 	}
 
-	if (differ)
-	{
-		status(_("FAILED"));
-		fail_count++;
-	}
-	else
-	{
-		status(_("ok"));
-		success_count++;
-	}
+	int expected_status = get_expected_status(test);
 
-	if (exit_status != 0)
-		log_child_failure(exit_status);
+	print_test_status(differ, exit_status, expected_status, -1);
 
 	status_end();
 }
@@ -1890,6 +2008,7 @@ help(void)
 	printf(_("                            (can be used multiple times to concatenate)\n"));
 	printf(_("  --srcdir=DIR              absolute path to source directory (for VPATH builds)\n"));
     printf(_(" --init-file=GPD_INIT_FILE  init file to be used for gpdiff\n"));
+    printf(_(" --expected-statuses-file=EXPECTED_STATUSES_FILE  file to read expected return codes for each test\n"));
 	printf(_("\n"));
 	printf(_("Options for using an existing installation:\n"));
 	printf(_("  --host=HOST               use postmaster running on HOST\n"));
@@ -1931,8 +2050,8 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
 		{"psqldir", required_argument, NULL, 16},
 		{"srcdir", required_argument, NULL, 17},
 		{"create-role", required_argument, NULL, 18},
-        	{"init-file", required_argument, NULL, 19},
-		{"tablespace", required_argument, NULL, 19},
+		{"init-file", required_argument, NULL, 19},
+		{"expected-statuses-file", required_argument, NULL, 20},
 		{NULL, 0, NULL, 0}
 	};
 
@@ -2025,20 +2144,13 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
 			case 18:
 				split_to_stringlist(strdup(optarg), ", ", &extraroles);
 				break;
-                        case 19:
-                	        initfile = strdup(optarg);
-                        break;
-                        case 20:
-			        tablespace = malloc(11 + strlen(optarg) + 1);
-			        if (!tablespace)
-			        {
-			        	fprintf(stderr, _("out of memory.\n"));
-			        	exit_nicely(2);
-			        }
-			        snprintf(tablespace, 11 + strlen(optarg) + 1,
-						"TABLESPACE %s", optarg);
-		        break;
-            		default:
+			case 19:
+				initfile = strdup(optarg);
+				break;
+			case 20:
+				expected_statuses_file = strdup(optarg);
+				break;
+			default:
 				/* getopt_long already emitted a complaint */
 				fprintf(stderr, _("\nTry \"%s -h\" for more information.\n"),
 						progname);
@@ -2105,6 +2217,7 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
 		run_single_test(sl->str, tfunc);
 	}
 
+	free_statuslist(&expected_statuses);
 	fclose(logfile);
 
 	/*