On Sun, Nov 13, 2011 at 09:05:41AM +0100, Holger Hans Peter Freyther wrote:
> On 11/13/2011 02:25 AM, Pablo Neira Ayuso wrote:
> 
> > Interesting, never played with this autotest stuff so far.
> 
> I saw it in GNU Smalltalk. Somehow the testsuite.at is too verbose and we
> would deserve an osmo.m4 macro but then we are in trouble in regard to
> installing this one. :}

Not sure I understood the problem. We can store the custom xyz.m4 file
under the m4/ directory and distribute it with the tarball, if needed.

> > Patch attached for this.
> 
> thanks, picked into the branch.
> 
> > Can we tell the tool to compare stdout but to ignore stderr? If so, we
> > can display the repeatable output in stdout and the non-repeatable output
> > in stderr.
> 
> Good idea, I comitted a patch to do this, would be awesome if you play with
> sending things to stdout/stderr.

Thanks for applying the patch. I made one patch for this.

> > Yes, with lots of timers, the expiration may not be done in time on
> > a loaded machine. If we can ignore the stderr output, we can put the
> > information about timers not expiring in time to stderr.
> 
> Not expiring at all should probably be counted as test failure?

You mean not expiring in time? I think so. If we use very few timers
for the test (like it is the case for 5 steps), it seems to me very
unlikely that we'll fail to make it in time. Well, if you launch some
very CPU intensive tasks I might be wrong, but I don't think this will
be the case for people running make check. After all, if we start getting
reports of people that run the test that bogusly fail, we can relax the
checkings.

Mreover it's a good idea to set some maximum wait time for the test to
finish via signals. Check the second patch attached to this email, apply
it if you like it.

Btw, please, don't remove me from the Cc, I check the list often but
my mail client has a filter to put emails directed to me (To or Cc) in
one specific folder, so I can prioritize emails that require some
reply from me.
>From 421e6a9d1d2184194acf72403ca2f8cc967a2bfd Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <[email protected]>
Date: Sun, 13 Nov 2011 17:22:33 +0100
Subject: [PATCH 1/2] tests: timer: use stderr for non-repeatable output

This makes happy gnu-autotest for the timer test.

We may still may fail if we run the test on a very heavy loaded
system, but given the amount of timers that we using for the
automatic test (only 32), this seems very unlikely to me.
---
 tests/timer/timer_test.c  |   17 +++++++++--------
 tests/timer/timer_test.ok |   21 ++-------------------
 2 files changed, 11 insertions(+), 27 deletions(-)

diff --git a/tests/timer/timer_test.c b/tests/timer/timer_test.c
index a01a9e5..72c07a9 100644
--- a/tests/timer/timer_test.c
+++ b/tests/timer/timer_test.c
@@ -73,8 +73,8 @@ static void main_timer_fired(void *data)
 	int i;
 
 	if (*step == timer_nsteps) {
-		printf("Main timer has finished, please, wait a bit for the "
-		       "final report.\n");
+		fprintf(stderr, "Main timer has finished, please, "
+				"wait a bit for the final report.\n");
 		return;
 	}
 	/* add 2^step pair of timers per step. */
@@ -96,7 +96,7 @@ static void main_timer_fired(void *data)
 		osmo_timer_schedule(&v->timer, seconds, 0);
 		llist_add(&v->head, &timer_test_list);
 	}
-	printf("added %d timers in step %u (expired=%u)\n",
+	fprintf(stderr, "added %d timers in step %u (expired=%u)\n",
 		add_in_this_step, *step, expired_timers);
 	total_timers += add_in_this_step;
 	osmo_timer_schedule(&main_timer, TIME_BETWEEN_STEPS, 0);
@@ -112,7 +112,8 @@ static void secondary_timer_fired(void *data)
 
 	timersub(&current, &v->stop, &res);
 	if (timercmp(&res, &precision, >)) {
-		printf("ERROR: timer %p has expired too late!\n", v->timer);
+		fprintf(stderr, "ERROR: timer %p has expired too late!\n",
+			v->timer);
 		too_late++;
 	}
 
@@ -120,7 +121,7 @@ static void secondary_timer_fired(void *data)
 	talloc_free(data);
 	expired_timers++;
 	if (expired_timers == total_timers) {
-		printf("test over: added=%u expired=%u too_late=%u \n",
+		fprintf(stdout, "test over: added=%u expired=%u too_late=%u \n",
 			total_timers, expired_timers, too_late);
 		exit(EXIT_SUCCESS);
 	}
@@ -155,8 +156,8 @@ int main(int argc, char *argv[])
 		}
 	}
 
-	printf("Running timer test for %u steps, accepting imprecision "
-	       "of %u.%.6u seconds\n",
+	fprintf(stdout, "Running timer test for %u steps, accepting "
+		"imprecision of %u.%.6u seconds\n",
 		timer_nsteps, TIMER_PRES_SECS, TIMER_PRES_USECS);
 
 	osmo_timer_schedule(&main_timer, 1, 0);
@@ -166,6 +167,6 @@ int main(int argc, char *argv[])
 		osmo_select_main(0);
 	}
 #else
-	printf("Select not supported on this platform!\n");
+	fprintf(stdout, "Select not supported on this platform!\n");
 #endif
 }
diff --git a/tests/timer/timer_test.ok b/tests/timer/timer_test.ok
index b1792f1..b4e0e11 100644
--- a/tests/timer/timer_test.ok
+++ b/tests/timer/timer_test.ok
@@ -1,19 +1,2 @@
-Running timer test for 16 steps, accepting imprecision of 0.010000 seconds
-added 1 timers in step 0 (expired=0)
-added 2 timers in step 1 (expired=0)
-added 4 timers in step 2 (expired=0)
-added 8 timers in step 3 (expired=0)
-added 16 timers in step 4 (expired=1)
-added 32 timers in step 5 (expired=4)
-added 64 timers in step 6 (expired=33)
-added 128 timers in step 7 (expired=87)
-added 256 timers in step 8 (expired=183)
-added 512 timers in step 9 (expired=466)
-added 1024 timers in step 10 (expired=970)
-added 2048 timers in step 11 (expired=1968)
-added 4096 timers in step 12 (expired=3994)
-added 8192 timers in step 13 (expired=8035)
-added 16384 timers in step 14 (expired=16324)
-added 32768 timers in step 15 (expired=32641)
-Main timer has finished, please, wait a bit for the final report.
-test over: added=65535 expired=65535 too_late=0 
+Running timer test for 5 steps, accepting imprecision of 0.010000 seconds
+test over: added=31 expired=31 too_late=0 
-- 
1.7.2.5

>From e56241a22748d17f718f98e24dbe5880f847934c Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <[email protected]>
Date: Sun, 13 Nov 2011 17:40:09 +0100
Subject: [PATCH 2/2] tests: timer: set maximum wait time to obtain test results

If the timer test takes more than 2 * (number of steps + 10), we
abort the test. This calculation is based on the maximum timeout
randomly set (10 seconds) plus the number of steps (some existing
timers may be reset in each step). We double this to have some
extra grace time to finish.
---
 tests/timer/timer_test.c |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/tests/timer/timer_test.c b/tests/timer/timer_test.c
index 72c07a9..3775151 100644
--- a/tests/timer/timer_test.c
+++ b/tests/timer/timer_test.c
@@ -24,6 +24,7 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <signal.h>
 #include <getopt.h>
 
 #include <osmocom/core/talloc.h>
@@ -137,10 +138,22 @@ static void secondary_timer_fired(void *data)
 	}
 }
 
+static void alarm_handler(int signum)
+{
+	fprintf(stderr, "ERROR: We took too long to run the timer test, "
+			"something seems broken, aborting.\n");
+	exit(EXIT_FAILURE);
+}
+
 int main(int argc, char *argv[])
 {
 	int c;
 
+	if (signal(SIGALRM, alarm_handler) == SIG_ERR) {
+		perror("cannot register signal handler");
+		exit(EXIT_FAILURE);
+	}
+
 	while ((c = getopt_long(argc, argv, "s:", NULL, NULL)) != -1) {
 	switch(c) {
 		case 's':
@@ -162,6 +175,12 @@ int main(int argc, char *argv[])
 
 	osmo_timer_schedule(&main_timer, 1, 0);
 
+	/* if the test takes too long, we may consider that the timer scheduler
+	 * has hung. We set some maximum wait time which is the double of the
+	 * maximum timeout randomly set (10 seconds, worst case) plus the
+	 * number of steps (since some of them are reset each step). */
+	alarm(2 * (10 + timer_nsteps));
+
 #ifdef HAVE_SYS_SELECT_H
 	while (1) {
 		osmo_select_main(0);
-- 
1.7.2.5

Reply via email to