Merge lp:~xnox/upstart/fix-qemu-test into lp:upstart

[Dimitri John Ledkov]

Originally the race was noticed in the Ubuntu Cloud image booted in qemu as part of autopkgtest runner.

Waiting for the pid of the test job, makes this race evident on my local host as well.

At first, I have tried to do the log checks in the pid greater than zero branch, but at that time the log was not written yet, hence I moved it later @ the test clean up. I am not sure how correct this is.

With this change & stgrabers initctl2dot merge fix, the autopkgtest pass as run in the jenkins environment.

[James Hunt (jamesodhunt)]

Hi Dmitrijs,

I think this is still subtly racy since it is possible that even in a failure scenario a partial log could be written.

The fix I'd suggest is:

(1) Put a check immediately after the 'nih_free (class)' in the TEST_ALLOC_SAFE block that performs the following checks:

- if test_alloc_failed is true, and the log file exists, just unlink it (there is no point in checking it as it might be only a partial log - we cannot know).

- if test_alloc_failed is not true, ensure the log file exists, check its contents and unlink it (essentially the block you have added).

(2) Re-instate the original 'assert0 (rmdir (dirname))' at the end of the test.

This way, there is no possibility of:

- the tests failing to detect that a full log was somehow written in a failure scenario.
- a failure resulting from a partial log being written in one of the failure iterations, followed by a successful write in the successful iteration (recall that the logs are appended to).

review: Needs Fixing

[Dimitri John Ledkov (xnox)]

Ok, I'll try that. Thanks.

[lp:~xnox/upstart/fix-qemu-test]

1393. By James Hunt on 2012-12-03

* Merge of lp:~xnox/upstart/fix-qemu-test.

[James Hunt (jamesodhunt)]

Thanks Dmitrijs! Merged.

review: Approve

[lp:~xnox/upstart/fix-qemu-test]

1394. By Dimitri John Ledkov on 2012-12-03

Fix the failed code-path

Unmerged revisions

=== modified file 'init/tests/test_job_process.c'
--- init/tests/test_job_process.c	2012-11-26 14:04:27 +0000
+++ init/tests/test_job_process.c	2012-12-03 09:43:23 +0000
@@ -4787,11 +4787,21 @@
 			assert0 (unlink (script));
 		} else {
 			TEST_GT (pid, 0);
+			TEST_EQ (waitpid (pid, &status, 0), pid);
 		}
 
 		TEST_ALLOC_SAFE {
 			/* May alloc space if there is log data */
 			nih_free (class);
+			if (!test_alloc_failed) {
+				TEST_GT (sprintf (filename, "%s/simple-test.log", dirname), 0);
+				output = fopen (filename, "r");
+				TEST_NE_P (output, NULL);
+				CHECK_FILE_EQ (output, "hello world\r\n", TRUE);
+				TEST_FILE_END (output);
+				TEST_EQ (fclose (output), 0);
+			}
+			unlink (filename);
 		}
 	}