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

Proposed by Steve Langasek on 2013-11-15
Status: Merged
Merged at revision: 1573
Proposed branch: lp:~vorlon/upstart/flaky-log-serialization-test
Merge into: lp:upstart
Diff against target: 160 lines (+25/-70)
3 files modified
ChangeLog (+11/-0)
init/tests/test_state.c (+14/-53)
test/test_util_common.h (+0/-17)
To merge this branch: bzr merge lp:~vorlon/upstart/flaky-log-serialization-test
Reviewer Review Type Date Requested Status
James Hunt 2013-11-15 Approve on 2013-11-18
Review via email: mp+195458@code.launchpad.net

Description of the change

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.

To post a comment you must log in.
1575. By Steve Langasek on 2013-11-15

drop TIMED_BLOCK() macro, no longer used.

James Hunt (jamesodhunt) wrote :

LGTM.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'ChangeLog'
2--- ChangeLog 2013-11-14 14:28:26 +0000
3+++ ChangeLog 2013-11-15 23:47:52 +0000
4@@ -1,3 +1,14 @@
5+2013-11-15 Steve Langasek <steve.langasek@ubuntu.com>
6+
7+ * init/tests/test_state.c: test_log_serialise():
8+ - simplify the test for unflushed logs; there's no need
9+ to let any writes to the logfile succeed before serializing,
10+ we only need one synchronization point to make sure we have a
11+ non-empty log buffer.
12+ - drop a spurious check of nih_io_watches at a point where its
13+ content cannot possibly have changed.
14+ * test/test_util_common.h: drop TIMED_BLOCK() macro, no longer used.
15+
16 2013-11-14 James Hunt <james.hunt@ubuntu.com>
17
18 * NEWS: Release 1.11
19
20=== modified file 'init/tests/test_state.c'
21--- init/tests/test_state.c 2013-11-13 14:25:38 +0000
22+++ init/tests/test_state.c 2013-11-15 23:47:52 +0000
23@@ -2044,7 +2044,7 @@
24 char filename[PATH_MAX];
25 pid_t pid;
26 int wait_fd;
27- int fds[2] = { -1 };
28+ int fd;
29 struct stat statbuf;
30 mode_t old_perms;
31 int status;
32@@ -2094,33 +2094,23 @@
33
34 TEST_EQ (openpty (&pty_master, &pty_slave, NULL, NULL, NULL), 0);
35
36- /* Provide a log file which is accessible initially */
37+ /* Make file inaccessible to ensure data cannot be written
38+ * and will thus be added to the unflushed buffer.
39+ */
40+ fd = open (filename, O_CREAT | O_EXCL, 0);
41+ TEST_NE (fd, -1);
42+ close (fd);
43+
44+ /* Set up logging that we know won't go anywhere yet */
45 log = log_new (NULL, filename, pty_master, 0);
46 TEST_NE_P (log, NULL);
47 TEST_FALSE (NIH_LIST_EMPTY (nih_io_watches));
48
49- assert0 (pipe (fds));
50-
51 TEST_CHILD_WAIT (pid, wait_fd) {
52- char *str = "hello\n";
53 char buf[1];
54- size_t str_len;
55-
56- str_len = strlen (str);
57-
58- close (fds[1]);
59+
60 close (pty_master);
61
62- /* Write initial data */
63- ret = write (pty_slave, str, str_len);
64- TEST_EQ ((size_t)ret, str_len);
65-
66- /* let parent continue */
67- TEST_CHILD_RELEASE (wait_fd);
68-
69- /* now wait for parent */
70- assert (read (fds[0], buf, 1) == 1);
71-
72 len = TEST_ARRAY_SIZE (test_data);
73 errno = 0;
74
75@@ -2128,6 +2118,9 @@
76 ret = write (pty_slave, test_data, len);
77 TEST_EQ ((size_t)ret, len);
78
79+ /* let parent continue */
80+ TEST_CHILD_RELEASE (wait_fd);
81+
82 /* keep child running until the parent is ready (to
83 * simulate a job which continues to run across
84 * a re-exec).
85@@ -2136,38 +2129,6 @@
86 }
87
88 close (pty_slave);
89- close (fds[0]);
90-
91- TEST_FALSE (NIH_LIST_EMPTY (nih_io_watches));
92-
93- got = FALSE;
94- TIMED_BLOCK (5) {
95- TEST_FORCE_WATCH_UPDATE ();
96- if (! stat (filename, &statbuf)) {
97- got = TRUE;
98- break;
99- }
100- sleep (1);
101- }
102-
103- TEST_EQ (got, TRUE);
104-
105- /* save */
106- old_perms = statbuf.st_mode;
107-
108- /* Make file inaccessible to ensure data cannot be written
109- * and will thus be added to the unflushed buffer.
110- */
111- TEST_EQ (chmod (filename, 0x0), 0);
112-
113- /* Artificially stop us writing to the already open log file with
114- * perms 000.
115- */
116- close (log->fd);
117- log->fd = -1;
118-
119- /* release child */
120- assert (write (fds[1], "\n", 1) == 1);
121
122 /* Ensure that unflushed buffer contains data */
123 TEST_WATCH_UPDATE ();
124@@ -2193,7 +2154,7 @@
125 TEST_EQ (waitpid (pid, &status, 0), pid);
126
127 /* Restore access to allow log to be written on destruction */
128- TEST_EQ (chmod (filename, old_perms), 0);
129+ TEST_EQ (chmod (filename, 0644), 0);
130
131 nih_free (log);
132 nih_free (new_log);
133
134=== modified file 'test/test_util_common.h'
135--- test/test_util_common.h 2013-11-13 14:25:38 +0000
136+++ test/test_util_common.h 2013-11-15 23:47:52 +0000
137@@ -670,23 +670,6 @@
138 #define TEST_STR_ARRAY_NOT_CONTAINS(_array, _pattern) \
139 _TEST_STR_ARRAY_CONTAINS (_array, _pattern, TRUE)
140
141-
142-/**
143- * TIMED_BLOCK:
144- * @secs: Seconds to run for.
145- *
146- * Run a block of code repeatedly for @secs seconds.
147- * Have the loop call sleep(3) to avoid a busy wait.
148- **/
149-#define TIMED_BLOCK(secs) \
150- for (time_t _start_time = 0, _now = 0, _wait_secs = (time_t)secs;\
151- ; _now = time (NULL)) \
152- if (! _start_time) { \
153- _start_time = _now = time (NULL); \
154- } else if ((_start_time + _wait_secs) < _now) { \
155- break; \
156- } else
157-
158 extern int test_user_mode;
159
160 /* Prototypes */

Subscribers

People subscribed via source and target branches