Merge lp:~jamesodhunt/upstart/fix-test_state-test into lp:upstart

Proposed by James Hunt
Status: Rejected
Rejected by: James Hunt
Proposed branch: lp:~jamesodhunt/upstart/fix-test_state-test
Merge into: lp:upstart
Diff against target: 144 lines (+66/-4)
4 files modified
ChangeLog (+8/-0)
init/tests/test_state.c (+21/-4)
test/test_util_common.c (+34/-0)
test/test_util_common.h (+3/-0)
To merge this branch: bzr merge lp:~jamesodhunt/upstart/fix-test_state-test
Reviewer Review Type Date Requested Status
Dimitri John Ledkov Needs Fixing
Review via email: mp+195031@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Dimitri John Ledkov (xnox) wrote :

timed_check() is essentially exec once in a lifetime, this is because: start_time & secs_to_wait are local static variables, which are set first time timed_check() is called & are never reset to 0, once the timing is complete.

Thus this is better to be done as a preprocessor macro.

So instead of timed_check(), would it be possible to use TEST_WATCH_UPDATE_TIMEOUT_SECS(secs) where TEST_FORCE_WATCH_UPDATE() was currently used?

Additional nih_io_watches asserts & init are all good.

ps. it looks like TEST_FORCE_WATCH_UPDATE_TIMEOUT_SECS & TEST_FORCE_WATCH_UPDATE_TIMEOUT are the same as TEST_FORCE_WATCH_UPDATE, since all three call _TEST_WATCH_UPDATE(1, timeout) and timeout is not used when force is 1.

pss. please note that select in _TEST_WATCH_UPDATE is rounded up, and in some cases ( when !have_timed_waitpid() ) it's rounded up to a hallarious value of 1 600 seconds or some such. (virtualised PPAs with acient XEN kernel / hyper visor)

review: Needs Fixing
Revision history for this message
Dimitri John Ledkov (xnox) wrote :

Unmerged revisions

1570. By James Hunt

* init/tests/test_state.c: test_log_serialise():
  - Added extra checks on nih_io_watches.
  - Need to wait for logfile to be written to handle case where NIH
    encounters EAGAIN.
* test/test_util_common.c: Added timed_check() utility function.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'ChangeLog'
--- ChangeLog 2013-11-12 12:17:30 +0000
+++ ChangeLog 2013-11-13 11:07:42 +0000
@@ -1,3 +1,11 @@
12013-11-13 James Hunt <james.hunt@ubuntu.com>
2
3 * init/tests/test_state.c: test_log_serialise():
4 - Added extra checks on nih_io_watches.
5 - Need to wait for logfile to be written to handle case where NIH
6 encounters EAGAIN.
7 * test/test_util_common.c: Added timed_check() utility function.
8
12013-11-12 James Hunt <james.hunt@ubuntu.com>92013-11-12 James Hunt <james.hunt@ubuntu.com>
210
3 * extra/man/socket-event.7: PATH => SOCKET_PATH.11 * extra/man/socket-event.7: PATH => SOCKET_PATH.
412
=== modified file 'init/tests/test_state.c'
--- init/tests/test_state.c 2013-10-24 12:35:33 +0000
+++ init/tests/test_state.c 2013-11-13 11:07:42 +0000
@@ -2048,12 +2048,15 @@
2048 struct stat statbuf;2048 struct stat statbuf;
2049 mode_t old_perms;2049 mode_t old_perms;
2050 int status;2050 int status;
2051 int got;
20512052
2052 conf_init ();2053 conf_init ();
2054 nih_io_init ();
2053 log_unflushed_init ();2055 log_unflushed_init ();
20542056
2055 TEST_TRUE (NIH_LIST_EMPTY (conf_sources));2057 TEST_TRUE (NIH_LIST_EMPTY (conf_sources));
2056 TEST_TRUE (NIH_LIST_EMPTY (log_unflushed_files));2058 TEST_TRUE (NIH_LIST_EMPTY (log_unflushed_files));
2059 TEST_TRUE (NIH_LIST_EMPTY (nih_io_watches));
20572060
2058 TEST_GROUP ("Log serialisation and deserialisation");2061 TEST_GROUP ("Log serialisation and deserialisation");
20592062
@@ -2067,6 +2070,7 @@
20672070
2068 log = log_new (NULL, "/foo", pty_master, 0);2071 log = log_new (NULL, "/foo", pty_master, 0);
2069 TEST_NE_P (log, NULL);2072 TEST_NE_P (log, NULL);
2073 TEST_FALSE (NIH_LIST_EMPTY (nih_io_watches));
20702074
2071 json = log_serialise (log);2075 json = log_serialise (log);
2072 TEST_NE_P (json, NULL);2076 TEST_NE_P (json, NULL);
@@ -2086,11 +2090,14 @@
20862090
2087 TEST_FILENAME (filename);2091 TEST_FILENAME (filename);
20882092
2093 TEST_TRUE (NIH_LIST_EMPTY (nih_io_watches));
2094
2089 TEST_EQ (openpty (&pty_master, &pty_slave, NULL, NULL, NULL), 0);2095 TEST_EQ (openpty (&pty_master, &pty_slave, NULL, NULL, NULL), 0);
20902096
2091 /* Provide a log file which is accessible initially */2097 /* Provide a log file which is accessible initially */
2092 log = log_new (NULL, filename, pty_master, 0);2098 log = log_new (NULL, filename, pty_master, 0);
2093 TEST_NE_P (log, NULL);2099 TEST_NE_P (log, NULL);
2100 TEST_FALSE (NIH_LIST_EMPTY (nih_io_watches));
20942101
2095 assert0 (pipe (fds));2102 assert0 (pipe (fds));
20962103
@@ -2131,10 +2138,19 @@
2131 close (pty_slave);2138 close (pty_slave);
2132 close (fds[0]);2139 close (fds[0]);
21332140
2134 /* Slurp the childs initial output */2141 TEST_FALSE (NIH_LIST_EMPTY (nih_io_watches));
2135 TEST_FORCE_WATCH_UPDATE ();2142
21362143 got = FALSE;
2137 TEST_EQ (stat (filename, &statbuf), 0);2144 while (timed_check (5)) {
2145 TEST_FORCE_WATCH_UPDATE ();
2146 if (! stat (filename, &statbuf)) {
2147 got = TRUE;
2148 break;
2149 }
2150 sleep (1);
2151 }
2152
2153 TEST_EQ (got, TRUE);
21382154
2139 /* save */2155 /* save */
2140 old_perms = statbuf.st_mode;2156 old_perms = statbuf.st_mode;
@@ -2181,6 +2197,7 @@
21812197
2182 nih_free (log);2198 nih_free (log);
2183 nih_free (new_log);2199 nih_free (new_log);
2200 TEST_TRUE (NIH_LIST_EMPTY (nih_io_watches));
2184 TEST_EQ (unlink (filename), 0);2201 TEST_EQ (unlink (filename), 0);
21852202
2186 /*******************************/2203 /*******************************/
21872204
=== modified file 'test/test_util_common.c'
--- test/test_util_common.c 2013-11-12 12:17:30 +0000
+++ test/test_util_common.c 2013-11-13 11:07:42 +0000
@@ -1002,3 +1002,37 @@
10021002
1003 }1003 }
1004}1004}
1005
1006/**
1007 * timed_check:
1008 * @secs: Seconds to run for.
1009 *
1010 * Return TRUE for @secs seconds. Place in a while loop to call a block
1011 * of code repeatedly for that duration (have the loop call sleep(3) to
1012 * avoid a busy wait!).
1013 *
1014 * Returns: TRUE whilst @secs timeout has not been reached, else FALSE.
1015 **/
1016int
1017timed_check (int secs)
1018{
1019 static time_t start_time = 0;
1020 static int secs_to_wait = 0;
1021 time_t now;
1022
1023 if (! secs)
1024 return FALSE;
1025
1026 if (! start_time)
1027 start_time = time (NULL);
1028
1029 if (! secs_to_wait)
1030 secs_to_wait = secs;
1031
1032 now = time (NULL);
1033
1034 if (now > (start_time + secs_to_wait))
1035 return FALSE;
1036
1037 return TRUE;
1038}
10051039
=== modified file 'test/test_util_common.h'
--- test/test_util_common.h 2013-11-12 12:17:30 +0000
+++ test/test_util_common.h 2013-11-13 11:07:42 +0000
@@ -756,4 +756,7 @@
756756
757void test_common_cleanup (void);757void test_common_cleanup (void);
758758
759int timed_check (int secs)
760 __attribute__ ((warn_unused_result));
761
759#endif /* TEST_UTIL_COMMON_H */762#endif /* TEST_UTIL_COMMON_H */

Subscribers

People subscribed via source and target branches