Merge lp:~vorlon/upstart/flaky-log-serialization-test into lp:upstart

[Steve Langasek]

A proposal to address the test failure seen here in jenkins:

  https://jenkins.qa.ubuntu.com/view/Trusty/view/AutoPkgTest/job/trusty-adt-upstart/14/ARCH=amd64,label=adt/

I don't have an explanation for why the test is failing the way it did here,
but in point of fact, the failure is entirely irrelevant to the point of the
test, which is for testing serialization of unflushed logs. We can get
information about the handling of unflushed logs without the complex
synchronization points in the current code.

[lp:~vorlon/upstart/flaky-log-serialization-test]

1575. By Steve Langasek on 2013-11-15

drop TIMED_BLOCK() macro, no longer used.

[James Hunt (jamesodhunt)]

LGTM.

review: Approve

=== modified file 'ChangeLog'
--- ChangeLog	2013-11-14 14:28:26 +0000
+++ ChangeLog	2013-11-15 23:47:52 +0000
@@ -1,3 +1,14 @@
[phone]  Steve Langasek  <[e-mail]>
+
+	* init/tests/test_state.c: test_log_serialise():
+	  - simplify the test for unflushed logs; there's no need
+	    to let any writes to the logfile succeed before serializing,
+	    we only need one synchronization point to make sure we have a
+	    non-empty log buffer.
+	  - drop a spurious check of nih_io_watches at a point where its
+	    content cannot possibly have changed.
+	* test/test_util_common.h: drop TIMED_BLOCK() macro, no longer used.
+
 2013-11-14  James Hunt  <[e-mail]>
 
 	* NEWS: Release 1.11
 
=== modified file 'init/tests/test_state.c'
--- init/tests/test_state.c	2013-11-13 14:25:38 +0000
+++ init/tests/test_state.c	2013-11-15 23:47:52 +0000
@@ -2044,7 +2044,7 @@
 	char             filename[PATH_MAX];
 	pid_t            pid;
 	int              wait_fd;
-	int              fds[2] = { -1 };
+	int              fd;
 	struct stat      statbuf;
 	mode_t           old_perms;
 	int              status;
@@ -2094,33 +2094,23 @@
 
 	TEST_EQ (openpty (&pty_master, &pty_slave, NULL, NULL, NULL), 0);
 
-	/* Provide a log file which is accessible initially */
+	/* Make file inaccessible to ensure data cannot be written
+	 * and will thus be added to the unflushed buffer.
+	 */
+	fd = open (filename, O_CREAT | O_EXCL, 0);
+	TEST_NE (fd, -1);
+	close (fd);
+
+	/* Set up logging that we know won't go anywhere yet */
 	log = log_new (NULL, filename, pty_master, 0);
 	TEST_NE_P (log, NULL);
 	TEST_FALSE (NIH_LIST_EMPTY (nih_io_watches));
 
-	assert0 (pipe (fds));
-
 	TEST_CHILD_WAIT (pid, wait_fd) {
-		char   *str = "hello\n";
 		char    buf[1];
-		size_t  str_len;
-
-		str_len = strlen (str);
-
-		close (fds[1]);
+
 		close (pty_master);
 
-		/* Write initial data */
-		ret = write (pty_slave, str, str_len);
-		TEST_EQ ((size_t)ret, str_len);
-
-		/* let parent continue */
-		TEST_CHILD_RELEASE (wait_fd);
-
-		/* now wait for parent */
-		assert (read (fds[0], buf, 1) == 1);
-
 		len = TEST_ARRAY_SIZE (test_data);
 		errno = 0;
 
@@ -2128,6 +2118,9 @@
 		ret = write (pty_slave, test_data, len);
 		TEST_EQ ((size_t)ret, len);
 
+		/* let parent continue */
+		TEST_CHILD_RELEASE (wait_fd);
+
 		/* keep child running until the parent is ready (to
 		 * simulate a job which continues to run across
 		 * a re-exec).
@@ -2136,38 +2129,6 @@
 	}
 
 	close (pty_slave);
-	close (fds[0]);
-
-	TEST_FALSE (NIH_LIST_EMPTY (nih_io_watches));
-
-	got = FALSE;
-	TIMED_BLOCK (5) {
-		TEST_FORCE_WATCH_UPDATE ();
-		if (! stat (filename, &statbuf)) {
-			got = TRUE;
-			break;
-		}
-		sleep (1);
-	}
-
-	TEST_EQ (got, TRUE);
-
-	/* save */
-	old_perms = statbuf.st_mode;
-
-	/* Make file inaccessible to ensure data cannot be written
-	 * and will thus be added to the unflushed buffer.
-	 */
-	TEST_EQ (chmod (filename, 0x0), 0);
-
-	/* Artificially stop us writing to the already open log file with
-	 * perms 000.
-	 */
-	close (log->fd);
-	log->fd = -1;
-
-	/* release child */
-	assert (write (fds[1], "\n", 1) == 1);
 
 	/* Ensure that unflushed buffer contains data */
 	TEST_WATCH_UPDATE ();
@@ -2193,7 +2154,7 @@
 	TEST_EQ (waitpid (pid, &status, 0), pid);
 
 	/* Restore access to allow log to be written on destruction */
-	TEST_EQ (chmod (filename, old_perms), 0);
+	TEST_EQ (chmod (filename, 0644), 0);
 
 	nih_free (log);
 	nih_free (new_log);
 
=== modified file 'test/test_util_common.h'
--- test/test_util_common.h	2013-11-13 14:25:38 +0000
+++ test/test_util_common.h	2013-11-15 23:47:52 +0000
@@ -670,23 +670,6 @@
 #define TEST_STR_ARRAY_NOT_CONTAINS(_array, _pattern)                \
         _TEST_STR_ARRAY_CONTAINS (_array, _pattern, TRUE)
 
-
-/**
- * TIMED_BLOCK:
- * @secs: Seconds to run for.
- *
- * Run a block of code repeatedly for @secs seconds.
- * Have the loop call sleep(3) to avoid a busy wait.
- **/
-#define TIMED_BLOCK(secs)                                                \
-	for (time_t _start_time = 0, _now = 0, _wait_secs = (time_t)secs;\
-		; _now = time (NULL))                                    \
-		if (! _start_time) {                                     \
-			_start_time = _now = time (NULL);                \
-		} else if ((_start_time + _wait_secs) < _now) {          \
-			break;                                           \
-		} else
-
 extern int test_user_mode;
 
 /* Prototypes */