Steve Langasek has proposed merging 
lp:~vorlon/upstart/flaky-log-serialization-test into lp:upstart.

Requested reviews:
  Upstart Reviewers (upstart-reviewers)

For more details, see:
https://code.launchpad.net/~vorlon/upstart/flaky-log-serialization-test/+merge/195458

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.
-- 
https://code.launchpad.net/~vorlon/upstart/flaky-log-serialization-test/+merge/195458
Your team Upstart Reviewers is requested to review the proposed merge of 
lp:~vorlon/upstart/flaky-log-serialization-test into lp:upstart.
=== modified file 'ChangeLog'
--- ChangeLog	2013-11-14 14:28:26 +0000
+++ ChangeLog	2013-11-15 23:43:37 +0000
@@ -1,3 +1,13 @@
+2013-11-15  Steve Langasek  <[email protected]>
+
+	* 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.
+
 2013-11-14  James Hunt  <[email protected]>
 
 	* 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:43:37 +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);

-- 
upstart-devel mailing list
[email protected]
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/upstart-devel

Reply via email to