All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH lttng-tools 1/8] Fix: tests: don't use pidof to wait for test apps
       [not found] <1416433232-20080-1-git-send-email-mathieu.desnoyers@efficios.com>
@ 2014-11-19 21:40 ` Mathieu Desnoyers
  2014-11-19 21:40 ` [PATCH lttng-tools 2/8] Fix: test flaky sleep and wait patterns Mathieu Desnoyers
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Mathieu Desnoyers @ 2014-11-19 21:40 UTC (permalink / raw)
  To: jgalar; +Cc: lttng-dev

Use the bash shell "wait" to wait for all background tasks rather than
the racy "pidof". Inded, it's possible that applications have been
forked, but not executed yet, when pidof is done, which would therefore
miss applications. Using "wait" from the shell solves this.

If we want to be really strict, we should have sessiond, consumerd, and
relayd export a file containing their own PID, and wait for this instead
of using pidof. But this will be for another fix.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 .../regression/tools/snapshots/test_ust_streaming  | 27 ++++++++++++----------
 tests/regression/tools/snapshots/ust_test          | 14 ++++++-----
 .../tools/streaming/test_high_throughput_limits    | 14 ++++-------
 tests/regression/ust/buffers-pid/test_buffers_pid  | 17 ++++----------
 .../ust/high-throughput/test_high_throughput       |  7 +++---
 tests/regression/ust/nprocesses/test_nprocesses    | 11 ++++-----
 .../test_periodical_metadata_flush                 | 24 +++++++------------
 7 files changed, 46 insertions(+), 68 deletions(-)

diff --git a/tests/regression/tools/snapshots/test_ust_streaming b/tests/regression/tools/snapshots/test_ust_streaming
index 632a563..c0d98c2 100755
--- a/tests/regression/tools/snapshots/test_ust_streaming
+++ b/tests/regression/tools/snapshots/test_ust_streaming
@@ -72,6 +72,15 @@ function start_trace_app()
 	rm -f $tmp_file
 }
 
+function stop_trace_app()
+{
+	diag "Killing $TESTAPP_NAME"
+	PID_APP=`pidof $TESTAPP_NAME`
+	kill $PID_APP >/dev/null 2>&1
+	diag "Waiting on $TESTAPP_NAME"
+	wait
+}
+
 # Test a snapshot using a default name for the output destination.
 function test_ust_default_name_with_del()
 {
@@ -90,6 +99,7 @@ function test_ust_default_name_with_del()
 	echo $TRACE_PATH/$HOSTNAME/snapshot-1
 	validate_trace $EVENT_NAME $TRACE_PATH/$HOSTNAME/snapshot-1*
 	if [ $? -ne 0 ]; then
+		stop_trace_app
 		return $?
 	fi
 
@@ -100,15 +110,14 @@ function test_ust_default_name_with_del()
 	# Validate test with the next ID since a del output was done prior.
 	validate_trace $EVENT_NAME $TRACE_PATH/$HOSTNAME/snapshot-2*
 	if [ $? -ne 0 ]; then
+		stop_trace_app
 		return $?
 	fi
 
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
 
-	diag "Killing $TESTAPP_NAME"
-	PID_APP=`pidof $TESTAPP_NAME`
-	kill $PID_APP >/dev/null 2>&1
+	stop_trace_app
 
 	return 0
 }
@@ -132,9 +141,7 @@ function test_ust_default_name()
 	validate_trace $EVENT_NAME $TRACE_PATH/$HOSTNAME/snapshot-1*
 	out=$?
 
-	diag "Killing $TESTAPP_NAME"
-	PID_APP=`pidof $TESTAPP_NAME`
-	kill $PID_APP >/dev/null 2>&1
+	stop_trace_app
 
 	return $out
 }
@@ -157,9 +164,7 @@ function test_ust_default_name_custom_uri()
 	validate_trace $EVENT_NAME $TRACE_PATH/$HOSTNAME/snapshot-1*
 	out=$?
 
-	diag "Killing $TESTAPP_NAME"
-	PID_APP=`pidof $TESTAPP_NAME`
-	kill $PID_APP >/dev/null 2>&1
+	stop_trace_app
 
 	return $out
 }
@@ -193,9 +198,7 @@ function test_ust_custom_name()
 		out=1
 	fi
 
-	diag "Killing $TESTAPP_NAME"
-	PID_APP=`pidof $TESTAPP_NAME`
-	kill $PID_APP >/dev/null 2>&1
+	stop_trace_app
 
 	return $out
 }
diff --git a/tests/regression/tools/snapshots/ust_test b/tests/regression/tools/snapshots/ust_test
index e727aa6..a35fbf3 100755
--- a/tests/regression/tools/snapshots/ust_test
+++ b/tests/regression/tools/snapshots/ust_test
@@ -63,11 +63,13 @@ function start_test_app()
 	rm -f $tmp_file
 }
 
-function kill_test_app()
+function stop_test_app()
 {
 	diag "Killing $TESTAPP_NAME"
 	PID_APP=`pidof $TESTAPP_NAME`
 	kill $PID_APP >/dev/null 2>&1
+	diag "Waiting on $TESTAPP_NAME"
+	wait
 }
 
 function snapshot_add_output ()
@@ -172,7 +174,7 @@ function test_ust_local_snapshot ()
 		break
 	fi
 
-	kill_test_app
+	stop_test_app
 }
 
 function test_ust_local_snapshot_max_size ()
@@ -220,7 +222,7 @@ function test_ust_local_snapshot_max_size ()
 		rm -rf $TRACE_PATH
 	fi
 
-	kill_test_app
+	stop_test_app
 }
 
 function test_ust_local_snapshot_large_metadata ()
@@ -286,7 +288,7 @@ function test_ust_per_uid_local_snapshot ()
 		break
 	fi
 
-	kill_test_app
+	stop_test_app
 }
 
 function test_ust_per_uid_local_snapshot_post_mortem ()
@@ -300,7 +302,7 @@ function test_ust_per_uid_local_snapshot_post_mortem ()
 
 	# Returns once the application has at least fired ONE tracepoint.
 	start_test_app
-	kill_test_app
+	stop_test_app
 
 	lttng_snapshot_record $SESSION_NAME
 	stop_lttng_tracing $SESSION_NAME
@@ -344,7 +346,7 @@ function test_ust_local_snapshots ()
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
 
-	kill_test_app
+	stop_test_app
 }
 
 plan_tests $NUM_TESTS
diff --git a/tests/regression/tools/streaming/test_high_throughput_limits b/tests/regression/tools/streaming/test_high_throughput_limits
index b2c8864..8ed2ce5 100755
--- a/tests/regression/tools/streaming/test_high_throughput_limits
+++ b/tests/regression/tools/streaming/test_high_throughput_limits
@@ -33,7 +33,7 @@ DEFAULT_IF="lo"
 
 TRACE_PATH=$(mktemp -d)
 
-NUM_TESTS=112
+NUM_TESTS=104
 
 source $TESTDIR/utils/utils.sh
 
@@ -95,14 +95,6 @@ function run_apps
 	done
 }
 
-function wait_apps
-{
-	while [ -n "$(pidof $TESTAPP_NAME)" ]; do
-		sleep 1
-	done
-	pass "Wait for applications to end"
-}
-
 function test_high_throughput
 {
 	NETWORK_URI="net://localhost"
@@ -110,7 +102,9 @@ function test_high_throughput
 	enable_ust_lttng_event $SESSION_NAME $EVENT_NAME
 	start_lttng_tracing $SESSION_NAME
 	run_apps
-	wait_apps
+	diag "Waiting for applications to end"
+	wait
+	pass "waiting done"
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
 	validate_event_count
diff --git a/tests/regression/ust/buffers-pid/test_buffers_pid b/tests/regression/ust/buffers-pid/test_buffers_pid
index c7ee2d8..974fb71 100755
--- a/tests/regression/ust/buffers-pid/test_buffers_pid
+++ b/tests/regression/ust/buffers-pid/test_buffers_pid
@@ -26,7 +26,7 @@ TESTAPP_PATH="$TESTDIR/utils/testapp"
 TESTAPP_NAME="gen-ust-events"
 TESTAPP_BIN="$TESTAPP_PATH/$TESTAPP_NAME/$TESTAPP_NAME"
 EVENT_NAME="tp:tptest"
-NUM_TESTS=58
+NUM_TESTS=59
 
 source $TESTDIR/utils/utils.sh
 
@@ -45,14 +45,6 @@ function enable_channel_per_pid()
 	ok $? "Enable channel $channel_name per PID for session $sess_name"
 }
 
-function wait_apps
-{
-	diag "Waiting for applications to end..."
-	while [ -n "$(pidof $TESTAPP_NAME)" ]; do
-		sleep 1
-	done
-}
-
 test_after_multiple_apps() {
 	local out
 	local i
@@ -95,8 +87,9 @@ test_before_multiple_apps() {
 	enable_ust_lttng_event $SESSION_NAME $EVENT_NAME "channel0"
 	start_lttng_tracing $SESSION_NAME
 
-	# At least hit one event
-	sleep 2
+	diag "Waiting for applications to end"
+	wait
+	pass "Waiting done"
 
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
@@ -112,8 +105,6 @@ test_before_multiple_apps() {
 		out=0
 	fi
 
-	wait_apps
-
 	return $out
 }
 
diff --git a/tests/regression/ust/high-throughput/test_high_throughput b/tests/regression/ust/high-throughput/test_high_throughput
index ea45051..101a2c0 100755
--- a/tests/regression/ust/high-throughput/test_high_throughput
+++ b/tests/regression/ust/high-throughput/test_high_throughput
@@ -49,10 +49,9 @@ for i in `seq 1 $NR_APP`; do
 	./$CURDIR/$BIN_NAME & >/dev/null 2>&1
 done
 
-while [ -n "$(pidof $BIN_NAME)" ]; do
-	sleep 0.5
-done
-pass "Wait for application end"
+diag "Waiting for applications to end"
+wait
+pass "Wait for applications to end"
 
 stop_lttng_tracing $SESSION_NAME
 destroy_lttng_session $SESSION_NAME
diff --git a/tests/regression/ust/nprocesses/test_nprocesses b/tests/regression/ust/nprocesses/test_nprocesses
index 1660c21..84d0ff7 100755
--- a/tests/regression/ust/nprocesses/test_nprocesses
+++ b/tests/regression/ust/nprocesses/test_nprocesses
@@ -75,12 +75,9 @@ destroy_lttng_session $SESSION_NAME
 
 rm -rf $TRACE_PATH
 
-while [ -n "$(pidof $TESTAPP_NAME)" ]; do
-	killall -q $TESTAPP_NAME >/dev/null 2>&1
-	sleep 0.5
-done
-
-
-pass "Kill all spawned applications"
+diag "Stopping all spawned applications"
+killall -q $TESTAPP_NAME >/dev/null 2>&1
+wait
+pass "Stopped all spawned applications"
 
 stop_lttng_sessiond
diff --git a/tests/regression/ust/periodical-metadata-flush/test_periodical_metadata_flush b/tests/regression/ust/periodical-metadata-flush/test_periodical_metadata_flush
index e836956..e419965 100755
--- a/tests/regression/ust/periodical-metadata-flush/test_periodical_metadata_flush
+++ b/tests/regression/ust/periodical-metadata-flush/test_periodical_metadata_flush
@@ -73,14 +73,6 @@ function enable_metadata_per_pid()
 	ok $? "Enable channel $channel_name per PID for session $sess_name"
 }
 
-function wait_apps
-{
-	diag "Waiting for applications to end..."
-	while [ -n "$(pidof $TESTAPP_NAME)" ]; do
-		sleep 1
-	done
-}
-
 function validate_trace()
 {
 	local out
@@ -149,11 +141,11 @@ test_after_app_pid() {
 	validate_trace
 	out=$?
 
+	killall -SIGKILL -q $TESTAPP_NAME
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
 
-	killall -SIGKILL -q $TESTAPP_NAME
-	wait_apps
+	wait
 
 	return $out
 }
@@ -188,11 +180,11 @@ test_before_app_pid() {
 	validate_trace
 	out=$?
 
+	killall -SIGKILL -q $TESTAPP_NAME
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
 
-	killall -SIGKILL -q $TESTAPP_NAME
-	wait_apps
+	wait
 
 	return $out
 }
@@ -223,11 +215,11 @@ test_after_app_uid() {
 	validate_trace
 	out=$?
 
+	killall -SIGKILL -q $TESTAPP_NAME
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
 
-	killall -SIGKILL -q $TESTAPP_NAME
-	wait_apps
+	wait
 
 	return $out
 }
@@ -261,11 +253,11 @@ test_before_app_uid() {
 	validate_trace
 	out=$?
 
+	killall -SIGKILL -q $TESTAPP_NAME
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
 
-	killall -SIGKILL -q $TESTAPP_NAME
-	wait_apps
+	wait
 
 	return $out
 }
-- 
2.1.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH lttng-tools 2/8] Fix: test flaky sleep and wait patterns
       [not found] <1416433232-20080-1-git-send-email-mathieu.desnoyers@efficios.com>
  2014-11-19 21:40 ` [PATCH lttng-tools 1/8] Fix: tests: don't use pidof to wait for test apps Mathieu Desnoyers
@ 2014-11-19 21:40 ` Mathieu Desnoyers
  2014-11-19 21:40 ` [PATCH lttng-tools 3/8] Document test anti-patterns Mathieu Desnoyers
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Mathieu Desnoyers @ 2014-11-19 21:40 UTC (permalink / raw)
  To: jgalar; +Cc: lttng-dev

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 tests/regression/tools/live/test_kernel            |  8 ++---
 tests/regression/tools/live/test_ust               |  8 ++---
 .../regression/tools/live/test_ust_tracefile_count |  8 ++---
 .../regression/tools/snapshots/test_ust_streaming  | 39 ++++++++++++----------
 tests/regression/tools/snapshots/ust_test          | 27 ++++++++-------
 tests/regression/tools/streaming/test_kernel       |  7 ----
 tests/regression/ust/java-jul/test_java_jul        | 10 ++++--
 tests/regression/ust/java-log4j/test_java_log4j    | 10 +++---
 tests/regression/ust/nprocesses/test_nprocesses    | 23 ++++++++-----
 .../ust/python-logging/test_python_logging         | 10 ++++--
 tests/stress/test_multi_sessions_per_uid_10app     |  4 +--
 .../test_multi_sessions_per_uid_5app_streaming     |  4 +--
 ...lti_sessions_per_uid_5app_streaming_kill_relayd |  8 ++---
 .../utils/testapp/gen-ust-events/gen-ust-events.c  | 14 +++++---
 tests/utils/utils.sh                               |  2 +-
 15 files changed, 95 insertions(+), 87 deletions(-)

diff --git a/tests/regression/tools/live/test_kernel b/tests/regression/tools/live/test_kernel
index ac4c19f..bf5e79c 100755
--- a/tests/regression/tools/live/test_kernel
+++ b/tests/regression/tools/live/test_kernel
@@ -63,18 +63,16 @@ else
 fi
 
 if [ -z $(pidof lt-$SESSIOND_BIN) ]; then
-	$DIR/../src/bin/lttng-sessiond/$SESSIOND_BIN --daemonize --quiet --consumerd32-path="$DIR/../src/bin/lttng-consumerd/lttng-consumerd" --consumerd64-path="$DIR/../src/bin/lttng-consumerd/lttng-consumerd"
+	$DIR/../src/bin/lttng-sessiond/$SESSIOND_BIN --background --quiet --consumerd32-path="$DIR/../src/bin/lttng-consumerd/lttng-consumerd" --consumerd64-path="$DIR/../src/bin/lttng-consumerd/lttng-consumerd"
 	if [ $? -eq 1 ]; then
 		echo "Fail to start lttng-sessiond"
 		exit 1
 	fi
-	# Wait for sessiond to bootstrap
-	sleep 2
 fi
 
-opt="-o $TRACE_PATH"
+opt="--background -o $TRACE_PATH"
 if [ -z $(pidof lt-$RELAYD_BIN) ]; then
-	$DIR/../src/bin/lttng-relayd/$RELAYD_BIN $opt >/dev/null 2>&1 &
+	$DIR/../src/bin/lttng-relayd/$RELAYD_BIN $opt >/dev/null 2>&1
 	if [ $? -eq 1 ]; then
 		echo "Fail to start lttng-relayd (opt: $opt)"
 		return 1
diff --git a/tests/regression/tools/live/test_ust b/tests/regression/tools/live/test_ust
index 2eac48c..4026bba 100755
--- a/tests/regression/tools/live/test_ust
+++ b/tests/regression/tools/live/test_ust
@@ -60,18 +60,16 @@ function clean_live_tracing()
 }
 
 if [ -z $(pidof lt-$SESSIOND_BIN) ]; then
-	$DIR/../src/bin/lttng-sessiond/$SESSIOND_BIN --daemonize --quiet --consumerd32-path="$DIR/../src/bin/lttng-consumerd/lttng-consumerd" --consumerd64-path="$DIR/../src/bin/lttng-consumerd/lttng-consumerd"
+	$DIR/../src/bin/lttng-sessiond/$SESSIOND_BIN --background --quiet --consumerd32-path="$DIR/../src/bin/lttng-consumerd/lttng-consumerd" --consumerd64-path="$DIR/../src/bin/lttng-consumerd/lttng-consumerd"
 	if [ $? -eq 1 ]; then
 		echo "Fail to start lttng-sessiond"
 		exit 1
 	fi
-	# Wait for sessiond to bootstrap
-	sleep 2
 fi
 
-opt="-o $TRACE_PATH"
+opt="-o $TRACE_PATH --background"
 if [ -z $(pidof lt-$RELAYD_BIN) ]; then
-	$DIR/../src/bin/lttng-relayd/$RELAYD_BIN $opt >/dev/null 2>&1 &
+	$DIR/../src/bin/lttng-relayd/$RELAYD_BIN $opt >/dev/null 2>&1
 	if [ $? -eq 1 ]; then
 		echo "Fail to start lttng-relayd (opt: $opt)"
 		return 1
diff --git a/tests/regression/tools/live/test_ust_tracefile_count b/tests/regression/tools/live/test_ust_tracefile_count
index f90cbd1..3ef2ee1 100755
--- a/tests/regression/tools/live/test_ust_tracefile_count
+++ b/tests/regression/tools/live/test_ust_tracefile_count
@@ -61,18 +61,16 @@ function clean_live_tracing()
 }
 
 if [ -z $(pidof lt-$SESSIOND_BIN) ]; then
-	$DIR/../src/bin/lttng-sessiond/$SESSIOND_BIN --daemonize --quiet --consumerd32-path="$DIR/../src/bin/lttng-consumerd/lttng-consumerd" --consumerd64-path="$DIR/../src/bin/lttng-consumerd/lttng-consumerd"
+	$DIR/../src/bin/lttng-sessiond/$SESSIOND_BIN --background --quiet --consumerd32-path="$DIR/../src/bin/lttng-consumerd/lttng-consumerd" --consumerd64-path="$DIR/../src/bin/lttng-consumerd/lttng-consumerd"
 	if [ $? -eq 1 ]; then
 		echo "Fail to start lttng-sessiond"
 		exit 1
 	fi
-	# Wait for sessiond to bootstrap
-	sleep 2
 fi
 
-opt="-o $TRACE_PATH"
+opt="-o $TRACE_PATH --background"
 if [ -z $(pidof lt-$RELAYD_BIN) ]; then
-	$DIR/../src/bin/lttng-relayd/$RELAYD_BIN $opt >/dev/null 2>&1 &
+	$DIR/../src/bin/lttng-relayd/$RELAYD_BIN $opt >/dev/null 2>&1
 	if [ $? -eq 1 ]; then
 		echo "Fail to start lttng-relayd (opt: $opt)"
 		return 1
diff --git a/tests/regression/tools/snapshots/test_ust_streaming b/tests/regression/tools/snapshots/test_ust_streaming
index c0d98c2..52330a5 100755
--- a/tests/regression/tools/snapshots/test_ust_streaming
+++ b/tests/regression/tools/snapshots/test_ust_streaming
@@ -28,6 +28,7 @@ TESTAPP_NAME="gen-ust-events"
 TESTAPP_BIN="$TESTAPP_PATH/$TESTAPP_NAME/$TESTAPP_NAME"
 NR_ITER=2000000
 NR_USEC_WAIT=100
+APPS_PID=
 
 TRACE_PATH=$(mktemp -d)
 
@@ -55,13 +56,15 @@ function snapshot_add_output ()
 }
 
 # Start trace application and return once one event has been hit.
-function start_trace_app()
+function start_test_app()
 {
 	local tmp_file="/tmp/lttng_test_ust.42.file"
 
 	# Start application with a temporary file.
 	$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT $tmp_file &
-	ok $? "Start application to trace"
+	ret=$?
+	APPS_PID="${APPS_PID} ${!}"
+	ok $ret "Start application to trace"
 
 	# Wait for the application file to appear indicating that at least one
 	# tracepoint has been fired.
@@ -72,13 +75,13 @@ function start_trace_app()
 	rm -f $tmp_file
 }
 
-function stop_trace_app()
+function stop_test_apps()
 {
-	diag "Killing $TESTAPP_NAME"
-	PID_APP=`pidof $TESTAPP_NAME`
-	kill $PID_APP >/dev/null 2>&1
-	diag "Waiting on $TESTAPP_NAME"
-	wait
+	diag "Stopping $TESTAPP_NAME"
+	for p in ${APPS_PID}; do
+		kill ${p}
+		wait ${p} 2>&1
+	done
 }
 
 # Test a snapshot using a default name for the output destination.
@@ -90,7 +93,7 @@ function test_ust_default_name_with_del()
 	enable_ust_lttng_event $SESSION_NAME $EVENT_NAME $CHANNEL_NAME
 	start_lttng_tracing $SESSION_NAME
 
-	start_trace_app
+	start_test_app
 
 	snapshot_add_output $SESSION_NAME "net://localhost"
 	lttng_snapshot_record $SESSION_NAME
@@ -99,7 +102,7 @@ function test_ust_default_name_with_del()
 	echo $TRACE_PATH/$HOSTNAME/snapshot-1
 	validate_trace $EVENT_NAME $TRACE_PATH/$HOSTNAME/snapshot-1*
 	if [ $? -ne 0 ]; then
-		stop_trace_app
+		stop_test_apps
 		return $?
 	fi
 
@@ -110,14 +113,14 @@ function test_ust_default_name_with_del()
 	# Validate test with the next ID since a del output was done prior.
 	validate_trace $EVENT_NAME $TRACE_PATH/$HOSTNAME/snapshot-2*
 	if [ $? -ne 0 ]; then
-		stop_trace_app
+		stop_test_apps
 		return $?
 	fi
 
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
 
-	stop_trace_app
+	stop_test_apps
 
 	return 0
 }
@@ -131,7 +134,7 @@ function test_ust_default_name()
 	enable_ust_lttng_event $SESSION_NAME $EVENT_NAME $CHANNEL_NAME
 	start_lttng_tracing $SESSION_NAME
 
-	start_trace_app
+	start_test_app
 
 	snapshot_add_output $SESSION_NAME "net://localhost"
 	lttng_snapshot_record $SESSION_NAME
@@ -141,7 +144,7 @@ function test_ust_default_name()
 	validate_trace $EVENT_NAME $TRACE_PATH/$HOSTNAME/snapshot-1*
 	out=$?
 
-	stop_trace_app
+	stop_test_apps
 
 	return $out
 }
@@ -154,7 +157,7 @@ function test_ust_default_name_custom_uri()
 	enable_ust_lttng_event $SESSION_NAME $EVENT_NAME $CHANNEL_NAME
 	start_lttng_tracing $SESSION_NAME
 
-	start_trace_app
+	start_test_app
 
 	snapshot_add_output $SESSION_NAME "-C tcp://localhost:5342 -D tcp://localhost:5343"
 	lttng_snapshot_record $SESSION_NAME
@@ -164,7 +167,7 @@ function test_ust_default_name_custom_uri()
 	validate_trace $EVENT_NAME $TRACE_PATH/$HOSTNAME/snapshot-1*
 	out=$?
 
-	stop_trace_app
+	stop_test_apps
 
 	return $out
 }
@@ -181,7 +184,7 @@ function test_ust_custom_name()
 	enable_ust_lttng_event $SESSION_NAME $EVENT_NAME $CHANNEL_NAME
 	start_lttng_tracing $SESSION_NAME
 
-	start_trace_app
+	start_test_app
 
 	snapshot_add_output $SESSION_NAME "net://localhost" $name
 	lttng_snapshot_record $SESSION_NAME
@@ -198,7 +201,7 @@ function test_ust_custom_name()
 		out=1
 	fi
 
-	stop_trace_app
+	stop_test_apps
 
 	return $out
 }
diff --git a/tests/regression/tools/snapshots/ust_test b/tests/regression/tools/snapshots/ust_test
index a35fbf3..4ef9f58 100755
--- a/tests/regression/tools/snapshots/ust_test
+++ b/tests/regression/tools/snapshots/ust_test
@@ -27,6 +27,7 @@ TESTAPP_NAME="gen-ust-events"
 TESTAPP_BIN="$TESTAPP_PATH/$TESTAPP_NAME/$TESTAPP_NAME"
 NR_ITER=2000000
 NR_USEC_WAIT=100
+APPS_PID=
 
 NUM_TESTS=76
 
@@ -52,7 +53,9 @@ function start_test_app()
 
 	# Start application with a temporary file.
 	$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT $tmp_file &
-	ok $? "Start application to trace"
+	ret=$?
+	APPS_PID="${APPS_PID} ${!}"
+	ok $ret "Start application to trace"
 
 	# Wait for the application file to appear indicating that at least one
 	# tracepoint has been fired.
@@ -63,13 +66,13 @@ function start_test_app()
 	rm -f $tmp_file
 }
 
-function stop_test_app()
+function stop_test_apps()
 {
-	diag "Killing $TESTAPP_NAME"
-	PID_APP=`pidof $TESTAPP_NAME`
-	kill $PID_APP >/dev/null 2>&1
-	diag "Waiting on $TESTAPP_NAME"
-	wait
+	diag "Stopping $TESTAPP_NAME"
+	for p in ${APPS_PID}; do
+		kill ${p}
+		wait ${p} 2>&1
+	done
 }
 
 function snapshot_add_output ()
@@ -174,7 +177,7 @@ function test_ust_local_snapshot ()
 		break
 	fi
 
-	stop_test_app
+	stop_test_apps
 }
 
 function test_ust_local_snapshot_max_size ()
@@ -222,7 +225,7 @@ function test_ust_local_snapshot_max_size ()
 		rm -rf $TRACE_PATH
 	fi
 
-	stop_test_app
+	stop_test_apps
 }
 
 function test_ust_local_snapshot_large_metadata ()
@@ -288,7 +291,7 @@ function test_ust_per_uid_local_snapshot ()
 		break
 	fi
 
-	stop_test_app
+	stop_test_apps
 }
 
 function test_ust_per_uid_local_snapshot_post_mortem ()
@@ -302,7 +305,7 @@ function test_ust_per_uid_local_snapshot_post_mortem ()
 
 	# Returns once the application has at least fired ONE tracepoint.
 	start_test_app
-	stop_test_app
+	stop_test_apps
 
 	lttng_snapshot_record $SESSION_NAME
 	stop_lttng_tracing $SESSION_NAME
@@ -346,7 +349,7 @@ function test_ust_local_snapshots ()
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
 
-	stop_test_app
+	stop_test_apps
 }
 
 plan_tests $NUM_TESTS
diff --git a/tests/regression/tools/streaming/test_kernel b/tests/regression/tools/streaming/test_kernel
index 9877bb8..1e272c3 100755
--- a/tests/regression/tools/streaming/test_kernel
+++ b/tests/regression/tools/streaming/test_kernel
@@ -52,13 +52,6 @@ function test_kernel_before_start ()
 	sleep 1
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
-
-	# We can not predict _yet_ when the trace is available so we have to do a
-	# arbitratry sleep to validate the trace.
-	diag "Wait 3 seconds for the trace to be written on disk"
-	for i in `seq 1 3`; do
-		sleep 1
-	done
 }
 
 # Deactivated since this feature is not yet available where we can enable
diff --git a/tests/regression/ust/java-jul/test_java_jul b/tests/regression/ust/java-jul/test_java_jul
index bcde273..359fe05 100755
--- a/tests/regression/ust/java-jul/test_java_jul
+++ b/tests/regression/ust/java-jul/test_java_jul
@@ -99,7 +99,7 @@ function test_jul_before_start ()
 	start_lttng_tracing $SESSION_NAME
 
 	# Wait for the applications started in background
-	wait ${!}
+	wait
 
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
@@ -399,7 +399,8 @@ function test_jul_destroy_session()
 	# Run 5 times with a 1 second delay
 	run_app_background 0 1
 
-	sleep 1
+	# Wait for the applications started in background
+	wait
 
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
@@ -414,8 +415,11 @@ function test_jul_destroy_session()
 	enable_jul_lttng_event $SESSION_NAME $EVENT_NAME2
 	start_lttng_tracing $SESSION_NAME
 
+	# Run 5 times with a 1 second delay
+	run_app_background 0 1
+
 	# Wait for the applications started in background
-	wait ${!}
+	wait
 
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
diff --git a/tests/regression/ust/java-log4j/test_java_log4j b/tests/regression/ust/java-log4j/test_java_log4j
index b4846c3..173d508 100755
--- a/tests/regression/ust/java-log4j/test_java_log4j
+++ b/tests/regression/ust/java-log4j/test_java_log4j
@@ -100,7 +100,7 @@ function test_log4j_before_start ()
 	start_lttng_tracing $SESSION_NAME
 
 	# Wait for the applications started in background
-	wait ${!}
+	wait
 
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
@@ -399,8 +399,8 @@ function test_log4j_destroy_session()
 
 	# Run 5 times with a 1 second delay
 	run_app_background 0 1
-
-	sleep 1
+	# Wait for the applications started in background
+	wait
 
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
@@ -415,8 +415,10 @@ function test_log4j_destroy_session()
 	enable_log4j_lttng_event $SESSION_NAME $EVENT_NAME2
 	start_lttng_tracing $SESSION_NAME
 
+	# Run 5 times with a 1 second delay
+	run_app_background 0 1
 	# Wait for the applications started in background
-	wait ${!}
+	wait
 
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
diff --git a/tests/regression/ust/nprocesses/test_nprocesses b/tests/regression/ust/nprocesses/test_nprocesses
index 84d0ff7..396dab0 100755
--- a/tests/regression/ust/nprocesses/test_nprocesses
+++ b/tests/regression/ust/nprocesses/test_nprocesses
@@ -19,7 +19,7 @@ TEST_DESC="UST tracer - Generate $NUM_PROCESS process"
 
 CURDIR=$(dirname $0)/
 TESTDIR=$CURDIR/../../..
-NR_ITER=1000
+NR_ITER=-1	# infinite loop
 NR_USEC_WAIT=1000000
 TESTAPP_PATH="$TESTDIR/utils/testapp"
 TESTAPP_NAME="gen-ust-events"
@@ -28,6 +28,7 @@ SESSION_NAME="ust-nprocesses"
 EVENT_NAME="tp:tptest"
 TEST_WAIT_SEC=5
 NUM_TESTS=9
+APPS_PID=
 
 source $TESTDIR/utils/utils.sh
 
@@ -43,20 +44,24 @@ print_test_banner "$TEST_DESC"
 
 start_lttng_sessiond
 
-# Start test for 1000 seconds
+# Start tests. Each is an infinite tracing loop.
 
+diag "Starting $NUM_PROCESS test applications"
 for i in `seq 1 $NUM_PROCESS`
 do
 	$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT >/dev/null 2>&1 &
+	APPS_PID="${APPS_PID} ${!}"
 done
 
+diag "Waiting for applications to be registered to sessiond"
+
 reg_app_count=0
 while [ $reg_app_count -ne $NUM_PROCESS ]; do
 	listing=$($TESTDIR/../src/bin/lttng/$LTTNG_BIN list -u)
 	reg_app_count=$(echo -n $listing | sed "s#$TESTAPP_BIN#$TESTAPP_BIN\n#g" | grep "$TESTAPP_BIN" | wc -l)
 done
 
-pass "Trace validation"
+pass "All applications are registered to sessiond"
 
 TRACE_PATH=$(mktemp -d)
 
@@ -65,10 +70,8 @@ create_lttng_session $SESSION_NAME $TRACE_PATH
 enable_ust_lttng_event $SESSION_NAME $EVENT_NAME
 start_lttng_tracing $SESSION_NAME
 
-diag "Sleeping $TEST_WAIT_SEC seconds for tracing to start everywhere"
-diag "Warning: this arbitrary time can make the test fail on slower system"
-
-sleep $TEST_WAIT_SEC
+# We don't validate whether the applications have traced here, rather
+# just that they registered to sessiond (above).
 
 stop_lttng_tracing $SESSION_NAME
 destroy_lttng_session $SESSION_NAME
@@ -76,8 +79,10 @@ destroy_lttng_session $SESSION_NAME
 rm -rf $TRACE_PATH
 
 diag "Stopping all spawned applications"
-killall -q $TESTAPP_NAME >/dev/null 2>&1
-wait
+for p in ${APPS_PID}; do
+	kill ${p}
+	wait ${p} 2>/dev/null
+done
 pass "Stopped all spawned applications"
 
 stop_lttng_sessiond
diff --git a/tests/regression/ust/python-logging/test_python_logging b/tests/regression/ust/python-logging/test_python_logging
index b2fa6fb..cb960da 100755
--- a/tests/regression/ust/python-logging/test_python_logging
+++ b/tests/regression/ust/python-logging/test_python_logging
@@ -99,7 +99,7 @@ function test_python_before_start ()
 	start_lttng_tracing $SESSION_NAME
 
 	# Wait for the applications started in background
-	wait ${!}
+	wait
 
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
@@ -403,7 +403,8 @@ function test_python_destroy_session()
 	# Run 5 times with a 1 second delay
 	run_app_background 0 1
 
-	sleep 1
+	# Wait for the applications started in background
+	wait
 
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
@@ -418,8 +419,11 @@ function test_python_destroy_session()
 	enable_python_lttng_event $SESSION_NAME $EVENT_NAME2
 	start_lttng_tracing $SESSION_NAME
 
+	# Run 5 times with a 1 second delay
+	run_app_background 0 1
+
 	# Wait for the applications started in background
-	wait ${!}
+	wait
 
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
diff --git a/tests/stress/test_multi_sessions_per_uid_10app b/tests/stress/test_multi_sessions_per_uid_10app
index c960957..365519d 100755
--- a/tests/stress/test_multi_sessions_per_uid_10app
+++ b/tests/stress/test_multi_sessions_per_uid_10app
@@ -67,10 +67,8 @@ function start_sessiond()
 	if [ -z $(pidof lt-$SESSIOND_BIN) ]; then
 		# We have to start it like this so the ulimit -c is used by this
 		# process. Also, we collect any error message printed out.
-		$TESTDIR/../src/bin/lttng-sessiond/$SESSIOND_BIN --quiet --consumerd32-path="$TESTDIR/../src/bin/lttng-consumerd/lttng-consumerd" --consumerd64-path="$TESTDIR/../src/bin/lttng-consumerd/lttng-consumerd" >$LOG_FILE 2>&1 &
+		$TESTDIR/../src/bin/lttng-sessiond/$SESSIOND_BIN --quiet --background --consumerd32-path="$TESTDIR/../src/bin/lttng-consumerd/lttng-consumerd" --consumerd64-path="$TESTDIR/../src/bin/lttng-consumerd/lttng-consumerd" >$LOG_FILE 2>&1
 		status=$?
-		# Wait for sessiond to bootstrap
-		sleep 2
 		ok $status "Start session daemon"
 	fi
 }
diff --git a/tests/stress/test_multi_sessions_per_uid_5app_streaming b/tests/stress/test_multi_sessions_per_uid_5app_streaming
index eb68107..40c0a4d 100755
--- a/tests/stress/test_multi_sessions_per_uid_5app_streaming
+++ b/tests/stress/test_multi_sessions_per_uid_5app_streaming
@@ -89,10 +89,8 @@ function start_sessiond()
 	if [ -z $(pidof lt-$SESSIOND_BIN) ]; then
 		# We have to start it like this so the ulimit -c is used by this
 		# process. Also, we collect any error message printed out.
-		$TESTDIR/../src/bin/lttng-sessiond/$SESSIOND_BIN --quiet --consumerd32-path="$TESTDIR/../src/bin/lttng-consumerd/lttng-consumerd" --consumerd64-path="$TESTDIR/../src/bin/lttng-consumerd/lttng-consumerd" >$LOG_FILE_SESSIOND 2>&1 &
+		$TESTDIR/../src/bin/lttng-sessiond/$SESSIOND_BIN --quiet --background --consumerd32-path="$TESTDIR/../src/bin/lttng-consumerd/lttng-consumerd" --consumerd64-path="$TESTDIR/../src/bin/lttng-consumerd/lttng-consumerd" >$LOG_FILE_SESSIOND 2>&1
 		status=$?
-		# Wait for sessiond to bootstrap
-		sleep 2
 		ok $status "Start session daemon"
 	fi
 }
diff --git a/tests/stress/test_multi_sessions_per_uid_5app_streaming_kill_relayd b/tests/stress/test_multi_sessions_per_uid_5app_streaming_kill_relayd
index 4ff9276..fa41b4c 100755
--- a/tests/stress/test_multi_sessions_per_uid_5app_streaming_kill_relayd
+++ b/tests/stress/test_multi_sessions_per_uid_5app_streaming_kill_relayd
@@ -76,12 +76,10 @@ function start_sessiond()
 	if [ -z $(pidof lt-$SESSIOND_BIN) ]; then
 		# We have to start it like this so the ulimit -c is used by this
 		# process. Also, we collect any error message printed out.
-		#$TESTDIR/../src/bin/lttng-sessiond/$SESSIOND_BIN --quiet --consumerd32-path="$TESTDIR/../src/bin/lttng-consumerd/lttng-consumerd" --consumerd64-path="$TESTDIR/../src/bin/lttng-consumerd/lttng-consumerd" >$LOG_FILE_SESSIOND 2>&1 &
-		$TESTDIR/../src/bin/lttng-sessiond/$SESSIOND_BIN --verbose-consumer -vvv --consumerd32-path="$TESTDIR/../src/bin/lttng-consumerd/lttng-consumerd" --consumerd64-path="$TESTDIR/../src/bin/lttng-consumerd/lttng-consumerd" >$LOG_FILE_SESSIOND 2>&1 &
-		#$TESTDIR/../src/bin/lttng-sessiond/$SESSIOND_BIN --consumerd32-path="$TESTDIR/../src/bin/lttng-consumerd/lttng-consumerd" --consumerd64-path="$TESTDIR/../src/bin/lttng-consumerd/lttng-consumerd" >$LOG_FILE_SESSIOND 2>&1 &
+		#$TESTDIR/../src/bin/lttng-sessiond/$SESSIOND_BIN --quiet --background --consumerd32-path="$TESTDIR/../src/bin/lttng-consumerd/lttng-consumerd" --consumerd64-path="$TESTDIR/../src/bin/lttng-consumerd/lttng-consumerd" >$LOG_FILE_SESSIOND 2>&1
+		$TESTDIR/../src/bin/lttng-sessiond/$SESSIOND_BIN --verbose-consumer -vvv --background --consumerd32-path="$TESTDIR/../src/bin/lttng-consumerd/lttng-consumerd" --consumerd64-path="$TESTDIR/../src/bin/lttng-consumerd/lttng-consumerd" >$LOG_FILE_SESSIOND 2>&1
+		#$TESTDIR/../src/bin/lttng-sessiond/$SESSIOND_BIN --background --consumerd32-path="$TESTDIR/../src/bin/lttng-consumerd/lttng-consumerd" --consumerd64-path="$TESTDIR/../src/bin/lttng-consumerd/lttng-consumerd" >$LOG_FILE_SESSIOND 2>&1
 		status=$?
-		# Wait for sessiond to bootstrap
-		sleep 2
 		ok $status "Start session daemon"
 	fi
 }
diff --git a/tests/utils/testapp/gen-ust-events/gen-ust-events.c b/tests/utils/testapp/gen-ust-events/gen-ust-events.c
index 77d88db..42fa082 100644
--- a/tests/utils/testapp/gen-ust-events/gen-ust-events.c
+++ b/tests/utils/testapp/gen-ust-events/gen-ust-events.c
@@ -26,6 +26,7 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <unistd.h>
+#include <stdbool.h>
 
 #define TRACEPOINT_DEFINE
 #include "tp.h"
@@ -47,16 +48,20 @@ void create_file(const char *path)
 
 int main(int argc, char **argv)
 {
-	int i, netint;
+	unsigned int i, netint;
 	long values[] = { 1, 2, 3 };
 	char text[10] = "test";
 	double dbl = 2.0;
 	float flt = 2222.0;
-	unsigned int nr_iter = 100;
+	int nr_iter = 100;
 	useconds_t nr_usec = 0;
 	char *tmp_file_path = NULL;
+	bool file_created = false;
 
 	if (argc >= 2) {
+		/*
+		 * If nr_iter is negative, do an infinite tracing loop.
+		 */
 		nr_iter = atoi(argv[1]);
 	}
 
@@ -69,7 +74,7 @@ int main(int argc, char **argv)
 		tmp_file_path = argv[3];
 	}
 
-	for (i = 0; i < nr_iter; i++) {
+	for (i = 0; nr_iter < 0 || i < nr_iter; i++) {
 		netint = htonl(i);
 		tracepoint(tp, tptest, i, netint, values, text, strlen(text), dbl,
 				flt);
@@ -78,8 +83,9 @@ int main(int argc, char **argv)
 		 * First loop we create the file if asked to indicate that at least one
 		 * tracepoint has been hit.
 		 */
-		if (i == 0 && tmp_file_path) {
+		if (!file_created && tmp_file_path) {
 			create_file(tmp_file_path);
+			file_created = true;
 		}
 		usleep(nr_usec);
 	}
diff --git a/tests/utils/utils.sh b/tests/utils/utils.sh
index fe75461..c3d8bd4 100644
--- a/tests/utils/utils.sh
+++ b/tests/utils/utils.sh
@@ -311,7 +311,7 @@ function start_lttng_sessiond()
 		else
 			$DIR/../src/bin/lttng-sessiond/$SESSIOND_BIN --background --consumerd32-path="$DIR/../src/bin/lttng-consumerd/lttng-consumerd" --consumerd64-path="$DIR/../src/bin/lttng-consumerd/lttng-consumerd"
 		fi
-		#$DIR/../src/bin/lttng-sessiond/$SESSIOND_BIN --consumerd32-path="$DIR/../src/bin/lttng-consumerd/lttng-consumerd" --consumerd64-path="$DIR/../src/bin/lttng-consumerd/lttng-consumerd" --verbose-consumer >>/tmp/sessiond.log 2>&1 &
+		#$DIR/../src/bin/lttng-sessiond/$SESSIOND_BIN --background --consumerd32-path="$DIR/../src/bin/lttng-consumerd/lttng-consumerd" --consumerd64-path="$DIR/../src/bin/lttng-consumerd/lttng-consumerd" --verbose-consumer >>/tmp/sessiond.log 2>&1
 		status=$?
 		ok $status "Start session daemon"
 	fi
-- 
2.1.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH lttng-tools 3/8] Document test anti-patterns
       [not found] <1416433232-20080-1-git-send-email-mathieu.desnoyers@efficios.com>
  2014-11-19 21:40 ` [PATCH lttng-tools 1/8] Fix: tests: don't use pidof to wait for test apps Mathieu Desnoyers
  2014-11-19 21:40 ` [PATCH lttng-tools 2/8] Fix: test flaky sleep and wait patterns Mathieu Desnoyers
@ 2014-11-19 21:40 ` Mathieu Desnoyers
  2014-11-19 21:40 ` [PATCH lttng-tools 4/8] Fix: tests: add missing wait, document missing synchro Mathieu Desnoyers
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Mathieu Desnoyers @ 2014-11-19 21:40 UTC (permalink / raw)
  To: jgalar; +Cc: lttng-dev

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 tests/Makefile.am |  2 +-
 tests/README      | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 1 deletion(-)
 create mode 100644 tests/README

diff --git a/tests/Makefile.am b/tests/Makefile.am
index c14e733..2f1f1c3 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -15,7 +15,7 @@ if USE_PYTHON
 endif
 
 dist_noinst_SCRIPTS = run.sh unit_tests fast_regression long_regression root_regression with_bindings_regression
-EXTRA_DIST = run.sh unit_tests fast_regression long_regression root_regression with_bindings_regression
+EXTRA_DIST = run.sh unit_tests fast_regression long_regression root_regression with_bindings_regression README
 
 all-local:
 	@if [ x"$(srcdir)" != x"$(builddir)" ]; then \
diff --git a/tests/README b/tests/README
new file mode 100644
index 0000000..fc8630c
--- /dev/null
+++ b/tests/README
@@ -0,0 +1,63 @@
+* Test Anti-Patterns
+
+OK, there are a few patterns that have been found over and over in the
+testing code base which makes the tests flaky. Here is an incomplete
+list. Don't do that.
+
+1) Using pidof to wait for a background application (by name) to
+   disappear.
+
+   Why is it flaky ?
+
+   The application may be delayed after being forked, but not executed
+   yet. Therefore, pidof will not find it. Use "wait" instead.
+
+2) Using sleep as delay-based optimistic synchronization technique.
+
+   Why is it flaky ?
+
+   Everything that needs to happen before/after other things need to
+   be explicitly synchronized using e.g. a file (used as a flag).
+   Sleep is just an indicator of a minimum arbitrary delay, but
+   machine load and scheduling can actually mess up the real delay
+   between applications. Use explicit synchronization points. Never
+   sleep.
+
+3) Using killall on a background application.
+
+   Why is it flaky ?
+
+   Similarly to pidof, killall may run before the background application
+   executes, thus failing to find it. Store the application PID after it
+   it launched in background into a temporary variable for later use
+   by kill and wait.
+
+4) Using wait ${!} to wait for completion of many background
+   applications.
+
+   Why is it flaky ?
+
+   It just waits for the last application put in background. Use
+   "wait" to wait for all background applications.
+
+5) Forgetting wait at the end (or error return path) of a test phase
+   that has background applications.
+
+   Why is it flaky ?
+
+   Those application may interact with the following testing phases,
+   thus skewing the results.
+
+6) Not grepping into the entire code base for similar patterns.
+
+   When you find a problematic coding pattern, chances are it appears
+   elsewhere in the testing code base. Please fix it everywhere!
+
+7) Introducing a utility abstraction without changing all open coded
+   similar code path.
+
+   When an abstraction for e.g. starting and stopping the session daemon
+   is introduced as a utility (e.g. utils.sh), future changes will
+   assume that all the testing code base is using this abstraction.
+   Leaving a few custom open-coded sites of duplicated code around is a
+   good way to make it a pain to update the abstraction in the future.
-- 
2.1.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH lttng-tools 4/8] Fix: tests: add missing wait, document missing synchro
       [not found] <1416433232-20080-1-git-send-email-mathieu.desnoyers@efficios.com>
                   ` (2 preceding siblings ...)
  2014-11-19 21:40 ` [PATCH lttng-tools 3/8] Document test anti-patterns Mathieu Desnoyers
@ 2014-11-19 21:40 ` Mathieu Desnoyers
  2014-11-19 21:40 ` [PATCH lttng-tools 5/8] Fix: high throughput test: reset bw limit on sigterm Mathieu Desnoyers
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Mathieu Desnoyers @ 2014-11-19 21:40 UTC (permalink / raw)
  To: jgalar; +Cc: lttng-dev

Move all wait ${!} that target a single process to "wait", to minimize
the chances to forget some background process in the future.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 .../regression/tools/snapshots/test_ust_streaming  |  1 +
 tests/regression/tools/snapshots/ust_test          |  1 +
 tests/regression/tools/streaming/test_ust          |  9 ++-
 .../regression/ust/before-after/test_before_after  |  7 ++-
 tests/regression/ust/buffers-pid/test_buffers_pid  | 17 ++++--
 .../ust/high-throughput/test_high_throughput       |  2 +-
 tests/regression/ust/java-jul/test_java_jul        |  2 +
 tests/regression/ust/java-log4j/test_java_log4j    |  2 +
 .../ust/multi-session/test_multi_session           |  2 +-
 tests/regression/ust/nprocesses/test_nprocesses    |  5 ++
 .../test_periodical_metadata_flush                 | 68 +++++++++++++++++-----
 11 files changed, 93 insertions(+), 23 deletions(-)

diff --git a/tests/regression/tools/snapshots/test_ust_streaming b/tests/regression/tools/snapshots/test_ust_streaming
index 52330a5..ad0c56b 100755
--- a/tests/regression/tools/snapshots/test_ust_streaming
+++ b/tests/regression/tools/snapshots/test_ust_streaming
@@ -82,6 +82,7 @@ function stop_test_apps()
 		kill ${p}
 		wait ${p} 2>&1
 	done
+	APPS_PID=
 }
 
 # Test a snapshot using a default name for the output destination.
diff --git a/tests/regression/tools/snapshots/ust_test b/tests/regression/tools/snapshots/ust_test
index 4ef9f58..69feb94 100755
--- a/tests/regression/tools/snapshots/ust_test
+++ b/tests/regression/tools/snapshots/ust_test
@@ -73,6 +73,7 @@ function stop_test_apps()
 		kill ${p}
 		wait ${p} 2>&1
 	done
+	APPS_PID=
 }
 
 function snapshot_add_output ()
diff --git a/tests/regression/tools/streaming/test_ust b/tests/regression/tools/streaming/test_ust
index 4c7ac97..4807175 100755
--- a/tests/regression/tools/streaming/test_ust
+++ b/tests/regression/tools/streaming/test_ust
@@ -56,9 +56,11 @@ function test_ust_before_start ()
 	# Run 5 times with a 1 second delay
 	$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT >/dev/null 2>&1 &
 
+	#FIXME: racy missing synchro
+
 	start_lttng_tracing $SESSION_NAME
 	# Wait for the applications started in background
-	wait ${!}
+	wait
 
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
@@ -74,8 +76,13 @@ function test_ust_after_start ()
 	# Run 5 times with a 1 second delay
 	$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT >/dev/null 2>&1
 
+	#FIXME: racy missing synchro
+
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
+
+	# Wait for the applications started in background
+	wait
 }
 
 plan_tests $NUM_TESTS
diff --git a/tests/regression/ust/before-after/test_before_after b/tests/regression/ust/before-after/test_before_after
index 112f41a..1535f36 100755
--- a/tests/regression/ust/before-after/test_before_after
+++ b/tests/regression/ust/before-after/test_before_after
@@ -42,7 +42,8 @@ function test_before_apps()
 	enable_ust_lttng_event $SESSION_NAME $EVENT_NAME
 	start_lttng_tracing $SESSION_NAME
 
-	$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT
+	$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT &
+	wait
 	ok $? "Traced application stopped."
 
 	stop_lttng_tracing $SESSION_NAME
@@ -63,9 +64,11 @@ function test_after_apps()
 	$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT &
 	ok $? "Application started in background."
 
+	#FIXME: racy missing synchronization
+
 	start_lttng_tracing $SESSION_NAME
 
-	wait ${!}
+	wait
 
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
diff --git a/tests/regression/ust/buffers-pid/test_buffers_pid b/tests/regression/ust/buffers-pid/test_buffers_pid
index 974fb71..0258e89 100755
--- a/tests/regression/ust/buffers-pid/test_buffers_pid
+++ b/tests/regression/ust/buffers-pid/test_buffers_pid
@@ -58,9 +58,10 @@ test_after_multiple_apps() {
 	start_lttng_tracing $SESSION_NAME
 
 	for i in `seq 1 5`; do
-		$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT >/dev/null 2>&1
+		$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT >/dev/null 2>&1 &
 		ok $? "Start application $i for tracing"
 	done
+	wait
 
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
@@ -77,10 +78,12 @@ test_before_multiple_apps() {
 	diag "Start multiple applications BEFORE tracing is started"
 
 	for i in `seq 1 5`; do
-		$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT & >/dev/null 2>&1
+		$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT >/dev/null 2>&1 &
 		ok $? "Start application $i for tracing"
 	done
 
+	#FIXME: racy missing synchronization
+
 	# BEFORE application is spawned
 	create_lttng_session $SESSION_NAME $TRACE_PATH
 	enable_channel_per_pid $SESSION_NAME "channel0"
@@ -119,8 +122,9 @@ test_after_app() {
 	enable_ust_lttng_event $SESSION_NAME $EVENT_NAME "channel0"
 	start_lttng_tracing $SESSION_NAME
 
-	$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT
+	$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT &
 	ok $? "Start application to trace"
+	wait
 
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
@@ -144,9 +148,11 @@ test_before_app() {
 	$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT &
 	ok $? "Start application to trace"
 
+	#FIXME: racy missing synchronization
+
 	start_lttng_tracing $SESSION_NAME
 
-	wait ${!}
+	wait
 
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
@@ -190,8 +196,9 @@ test_multiple_channels() {
 	ok $? "Enable event $EVENT_NAME for session $SESSION_NAME in channel4"
 	start_lttng_tracing $SESSION_NAME
 
-	$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT
+	$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT &
 	ok $? "Start application to trace"
+	wait
 
 	stop_lttng_tracing $SESSION_NAME
 	trace_match_only $EVENT_NAME $[NR_ITER * 5] $TRACE_PATH
diff --git a/tests/regression/ust/high-throughput/test_high_throughput b/tests/regression/ust/high-throughput/test_high_throughput
index 101a2c0..1ee0ec1 100755
--- a/tests/regression/ust/high-throughput/test_high_throughput
+++ b/tests/regression/ust/high-throughput/test_high_throughput
@@ -46,7 +46,7 @@ enable_ust_lttng_event $SESSION_NAME $EVENT_NAME
 start_lttng_tracing $SESSION_NAME
 
 for i in `seq 1 $NR_APP`; do
-	./$CURDIR/$BIN_NAME & >/dev/null 2>&1
+	./$CURDIR/$BIN_NAME >/dev/null 2>&1 &
 done
 
 diag "Waiting for applications to end"
diff --git a/tests/regression/ust/java-jul/test_java_jul b/tests/regression/ust/java-jul/test_java_jul
index 359fe05..d16880f 100755
--- a/tests/regression/ust/java-jul/test_java_jul
+++ b/tests/regression/ust/java-jul/test_java_jul
@@ -96,6 +96,8 @@ function test_jul_before_start ()
 	# Run 5 times with a 1 second delay
 	run_app_background
 
+	#FIXME: racy missing synchronization
+
 	start_lttng_tracing $SESSION_NAME
 
 	# Wait for the applications started in background
diff --git a/tests/regression/ust/java-log4j/test_java_log4j b/tests/regression/ust/java-log4j/test_java_log4j
index 173d508..11845a0 100755
--- a/tests/regression/ust/java-log4j/test_java_log4j
+++ b/tests/regression/ust/java-log4j/test_java_log4j
@@ -97,6 +97,8 @@ function test_log4j_before_start ()
 	# Run 5 times with a 1 second delay
 	run_app_background
 
+	#FIXME: racy missing synchronization
+
 	start_lttng_tracing $SESSION_NAME
 
 	# Wait for the applications started in background
diff --git a/tests/regression/ust/multi-session/test_multi_session b/tests/regression/ust/multi-session/test_multi_session
index 19f8d42..e1ff9b0 100755
--- a/tests/regression/ust/multi-session/test_multi_session
+++ b/tests/regression/ust/multi-session/test_multi_session
@@ -44,7 +44,7 @@ test_multi_session() {
 	./$CURDIR/gen-nevents $NR_ITER &
 	ok $? "Start application to generate $NR_ITER events"
 
-	wait ${!}
+	wait
 	pass "Wait for events to record"
 
 	for i in `seq 0 3`; do
diff --git a/tests/regression/ust/nprocesses/test_nprocesses b/tests/regression/ust/nprocesses/test_nprocesses
index 396dab0..ca0f9d4 100755
--- a/tests/regression/ust/nprocesses/test_nprocesses
+++ b/tests/regression/ust/nprocesses/test_nprocesses
@@ -53,6 +53,8 @@ do
 	APPS_PID="${APPS_PID} ${!}"
 done
 
+#FIXME: racy missing synchronization
+
 diag "Waiting for applications to be registered to sessiond"
 
 reg_app_count=0
@@ -76,6 +78,8 @@ start_lttng_tracing $SESSION_NAME
 stop_lttng_tracing $SESSION_NAME
 destroy_lttng_session $SESSION_NAME
 
+#FIXME/TODO: add validation after fixing racy synchroniaation
+
 rm -rf $TRACE_PATH
 
 diag "Stopping all spawned applications"
@@ -83,6 +87,7 @@ for p in ${APPS_PID}; do
 	kill ${p}
 	wait ${p} 2>/dev/null
 done
+APPS_PID=
 pass "Stopped all spawned applications"
 
 stop_lttng_sessiond
diff --git a/tests/regression/ust/periodical-metadata-flush/test_periodical_metadata_flush b/tests/regression/ust/periodical-metadata-flush/test_periodical_metadata_flush
index e419965..53db813 100755
--- a/tests/regression/ust/periodical-metadata-flush/test_periodical_metadata_flush
+++ b/tests/regression/ust/periodical-metadata-flush/test_periodical_metadata_flush
@@ -28,6 +28,7 @@ EVENT_NAME="tp:tptest"
 BIN_NAME="gen-nevents"
 NUM_TESTS=38
 APP_TMP_FILE="/tmp/lttng_test_ust.42.file"
+APPS_PID=
 
 source $TESTDIR/utils/utils.sh
 
@@ -106,7 +107,9 @@ function start_trace_app()
 {
 	# Start application with a temporary file.
 	$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT $APP_TMP_FILE &
-	ok $? "Start application to trace"
+	ret=$?
+	APPS_PID="${APPS_PID} ${!}"
+	ok $ret "Start application to trace"
 }
 
 function start_check_trace_app()
@@ -115,9 +118,20 @@ function start_check_trace_app()
 	check_app_tmp_file
 }
 
+
+function wait_trace_apps()
+{
+	for p in ${APPS_PID}; do
+		wait ${p} 2>/dev/null
+	done
+	APPS_PID=
+}
+
 test_after_app_pid() {
 	local out
 
+	APPS_PID=
+
 	diag "Start application AFTER tracing is started"
 
 	create_lttng_session $SESSION_NAME $TRACE_PATH
@@ -133,7 +147,9 @@ test_after_app_pid() {
 	# Make sure the application does not generate any more data,
 	# thus ensuring that we are not flushing a packet concurrently
 	# with validate_trace.
-	killall -SIGSTOP -q $TESTAPP_NAME
+	for p in ${APPS_PID}; do
+		kill -s SIGSTOP ${p}
+	done
 
 	# Give time to the consumer to write inflight data.
 	sleep 2
@@ -141,11 +157,14 @@ test_after_app_pid() {
 	validate_trace
 	out=$?
 
-	killall -SIGKILL -q $TESTAPP_NAME
+	for p in ${APPS_PID}; do
+		kill -s SIGKILL ${p}
+		wait ${p} 2>/dev/null
+	done
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
 
-	wait
+	wait_trace_apps
 
 	return $out
 }
@@ -154,6 +173,8 @@ test_before_app_pid() {
 	local out
 	local tmp_file="/tmp/lttng_test_ust.42.file"
 
+	APPS_PID=
+
 	diag "Start application BEFORE tracing is started"
 
 	start_trace_app
@@ -172,7 +193,9 @@ test_before_app_pid() {
 	# Make sure the application does not generate any more data,
 	# thus ensuring that we are not flushing a packet concurrently
 	# with validate_trace.
-	killall -SIGSTOP -q $TESTAPP_NAME
+	for p in ${APPS_PID}; do
+		kill -s SIGSTOP ${p}
+	done
 
 	# Give time to the consumer to write inflight data.
 	sleep 2
@@ -180,11 +203,15 @@ test_before_app_pid() {
 	validate_trace
 	out=$?
 
-	killall -SIGKILL -q $TESTAPP_NAME
+	for p in ${APPS_PID}; do
+		kill -s SIGKILL ${p}
+		wait ${p} 2>/dev/null
+	done
+
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
 
-	wait
+	wait_trace_apps
 
 	return $out
 }
@@ -192,6 +219,8 @@ test_before_app_pid() {
 test_after_app_uid() {
 	local out
 
+	APPS_PID=
+
 	diag "Start application AFTER tracing is started"
 
 	create_lttng_session $SESSION_NAME $TRACE_PATH
@@ -207,7 +236,10 @@ test_after_app_uid() {
 	# Make sure the application does not generate any more data,
 	# thus ensuring that we are not flushing a packet concurrently
 	# with validate_trace.
-	killall -SIGSTOP -q $TESTAPP_NAME
+	for p in ${APPS_PID}; do
+		kill -s SIGSTOP ${p}
+	done
+
 
 	# Give time to the consumer to write inflight data.
 	sleep 2
@@ -215,11 +247,14 @@ test_after_app_uid() {
 	validate_trace
 	out=$?
 
-	killall -SIGKILL -q $TESTAPP_NAME
+	for p in ${APPS_PID}; do
+		kill -s SIGKILL ${p}
+		wait ${p} 2>/dev/null
+	done
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
 
-	wait
+	wait_trace_apps
 
 	return $out
 }
@@ -227,6 +262,8 @@ test_after_app_uid() {
 test_before_app_uid() {
 	local out
 
+	APPS_PID=
+
 	diag "Start application BEFORE tracing is started"
 
 	# Start application before tracing
@@ -245,7 +282,9 @@ test_before_app_uid() {
 	# Make sure the application does not generate any more data,
 	# thus ensuring that we are not flushing a packet concurrently
 	# with validate_trace.
-	killall -SIGSTOP -q $TESTAPP_NAME
+	for p in ${APPS_PID}; do
+		kill -s SIGSTOP ${p}
+	done
 
 	# Give time to the consumer to write inflight data.
 	sleep 2
@@ -253,11 +292,14 @@ test_before_app_uid() {
 	validate_trace
 	out=$?
 
-	killall -SIGKILL -q $TESTAPP_NAME
+	for p in ${APPS_PID}; do
+		kill -s SIGKILL ${p}
+		wait ${p} 2>/dev/null
+	done
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
 
-	wait
+	wait_trace_apps
 
 	return $out
 }
-- 
2.1.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH lttng-tools 5/8] Fix: high throughput test: reset bw limit on sigterm
       [not found] <1416433232-20080-1-git-send-email-mathieu.desnoyers@efficios.com>
                   ` (3 preceding siblings ...)
  2014-11-19 21:40 ` [PATCH lttng-tools 4/8] Fix: tests: add missing wait, document missing synchro Mathieu Desnoyers
@ 2014-11-19 21:40 ` Mathieu Desnoyers
  2014-11-19 21:40 ` [PATCH lttng-tools 6/8] Fix: tests: remove killall, add missing SIGTERM handlers Mathieu Desnoyers
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Mathieu Desnoyers @ 2014-11-19 21:40 UTC (permalink / raw)
  To: jgalar; +Cc: lttng-dev

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 tests/regression/tools/streaming/test_high_throughput_limits | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/regression/tools/streaming/test_high_throughput_limits b/tests/regression/tools/streaming/test_high_throughput_limits
index 8ed2ce5..9f75471 100755
--- a/tests/regression/tools/streaming/test_high_throughput_limits
+++ b/tests/regression/tools/streaming/test_high_throughput_limits
@@ -168,12 +168,14 @@ skip $isroot "Root access is needed to set bandwith limits. Skipping all tests."
 {
 
 	# Catch sigint and try to cleanup limits
+	trap interrupt_cleanup SIGTERM
 	trap interrupt_cleanup SIGINT
 
 	BW_LIMITS=(3200 1600 800 400 200 100 50 25)
 	for BW in ${BW_LIMITS[@]};
 	do
 		diag "Test high-throughput with bandwidth limit set to ${BW}kbits"
+
 		set_bw_limit $BW
 
 		start_lttng_sessiond
-- 
2.1.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH lttng-tools 6/8] Fix: tests: remove killall, add missing SIGTERM handlers
       [not found] <1416433232-20080-1-git-send-email-mathieu.desnoyers@efficios.com>
                   ` (4 preceding siblings ...)
  2014-11-19 21:40 ` [PATCH lttng-tools 5/8] Fix: high throughput test: reset bw limit on sigterm Mathieu Desnoyers
@ 2014-11-19 21:40 ` Mathieu Desnoyers
  2014-11-19 21:40 ` [PATCH lttng-tools 7/8] Fix: tests: wait output hide Terminate errors Mathieu Desnoyers
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Mathieu Desnoyers @ 2014-11-19 21:40 UTC (permalink / raw)
  To: jgalar; +Cc: lttng-dev

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 tests/stress/test_multi_sessions_per_uid_10app               |  9 ++++++++-
 tests/stress/test_multi_sessions_per_uid_5app_streaming      |  9 ++++++++-
 .../test_multi_sessions_per_uid_5app_streaming_kill_relayd   | 12 ++++++++++--
 3 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/tests/stress/test_multi_sessions_per_uid_10app b/tests/stress/test_multi_sessions_per_uid_10app
index 365519d..a86b724 100755
--- a/tests/stress/test_multi_sessions_per_uid_10app
+++ b/tests/stress/test_multi_sessions_per_uid_10app
@@ -24,6 +24,7 @@ NR_APP=10
 NR_SESSION=5
 NR_LOOP=1000
 COREDUMP_FILE=$(cat /proc/sys/kernel/core_pattern)
+APPS_PID=
 
 TEST_DESC="Stress test - $NR_SESSION sessions per UID with $NR_APP apps"
 
@@ -101,7 +102,11 @@ test_stress()
 function cleanup()
 {
 	diag "Cleaning up!"
-	killall -9 $LAUNCH_APP
+	for p in ${APPS_PID}; do
+		kill -s SIGKILL ${p}
+		wait ${p} 2>/dev/null
+	done
+	APPS_PID=
 	stop_lttng_sessiond
 }
 
@@ -113,6 +118,7 @@ function sighandler()
 }
 
 trap sighandler SIGINT
+trap sighandler SIGTERM
 
 # Make sure we collect a coredump if possible.
 ulimit -c unlimited
@@ -128,6 +134,7 @@ diag "Starting applications"
 
 # Start NR_APP applications script that will spawn apps non stop.
 ./$TESTDIR/stress/$LAUNCH_APP $NR_APP &
+APPS_PID="${APPS_PID} ${!}"
 
 TRACE_PATH=$(mktemp -d)
 
diff --git a/tests/stress/test_multi_sessions_per_uid_5app_streaming b/tests/stress/test_multi_sessions_per_uid_5app_streaming
index 40c0a4d..36a15d9 100755
--- a/tests/stress/test_multi_sessions_per_uid_5app_streaming
+++ b/tests/stress/test_multi_sessions_per_uid_5app_streaming
@@ -23,6 +23,7 @@ NR_SESSION=5
 NR_LOOP=1000
 COREDUMP_FILE=$(cat /proc/sys/kernel/core_pattern)
 NUM_TESTS=16
+APPS_PID=
 
 TEST_DESC="Stress test - $NR_SESSION sessions per UID streaming with $NR_APP apps"
 
@@ -140,7 +141,11 @@ test_stress()
 function cleanup()
 {
 	diag "Cleaning up!"
-	killall -9 $LAUNCH_APP
+	for p in ${APPS_PID}; do
+		kill -s SIGKILL ${p}
+		wait ${p} 2>/dev/null
+	done
+	APPS_PID=
 	stop_lttng_sessiond
 	stop_lttng_relayd
 }
@@ -153,6 +158,7 @@ function sighandler()
 }
 
 trap sighandler SIGINT
+trap sighandler SIGTERM
 
 # Make sure we collect a coredump if possible.
 ulimit -c unlimited
@@ -171,6 +177,7 @@ diag "Starting applications launcher"
 
 # Start NR_APP applications script that will spawn apps non stop.
 ./$TESTDIR/stress/$LAUNCH_APP $NR_APP &
+APPS_PID="${APPS_PID} ${!}"
 
 test_stress
 out=$?
diff --git a/tests/stress/test_multi_sessions_per_uid_5app_streaming_kill_relayd b/tests/stress/test_multi_sessions_per_uid_5app_streaming_kill_relayd
index fa41b4c..e01e6aa 100755
--- a/tests/stress/test_multi_sessions_per_uid_5app_streaming_kill_relayd
+++ b/tests/stress/test_multi_sessions_per_uid_5app_streaming_kill_relayd
@@ -24,6 +24,7 @@ NR_SESSION=5
 NR_LOOP=100000
 COREDUMP_FILE=$(cat /proc/sys/kernel/core_pattern)
 NUM_TESTS=16
+APPS_PID=
 
 TEST_DESC="Stress test - $NR_SESSION sessions per UID streaming with $NR_APP apps. The relayd is killed sporadically"
 
@@ -142,8 +143,11 @@ test_stress()
 function cleanup()
 {
 	diag "Cleaning up!"
-	killall -9 $LAUNCH_APP
-	killall -9 $KILL_RELAYD_HELPER
+	for p in ${APPS_PID}; do
+		kill ${p}
+		wait ${p} 2>/dev/null
+	done
+	APPS_PID=
 	stop_lttng_sessiond
 	stop_lttng_relayd
 }
@@ -156,6 +160,7 @@ function sighandler()
 }
 
 trap sighandler SIGINT
+trap sighandler SIGTERM
 
 # Make sure we collect a coredump if possible.
 ulimit -c unlimited
@@ -174,8 +179,11 @@ diag "Starting applications launcher"
 
 # Start NR_APP applications script that will spawn apps non stop.
 ./$TESTDIR/stress/$LAUNCH_APP $NR_APP &
+APPS_PID="${APPS_PID} ${!}"
+
 # Launch the helper script that will randomly kill the relayd at vitam eternam.
 ./$TESTDIR/stress/$KILL_RELAYD_HELPER 1 1 &
+APPS_PID="${APPS_PID} ${!}"
 
 test_stress
 out=$?
-- 
2.1.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH lttng-tools 7/8] Fix: tests: wait output hide Terminate errors
       [not found] <1416433232-20080-1-git-send-email-mathieu.desnoyers@efficios.com>
                   ` (5 preceding siblings ...)
  2014-11-19 21:40 ` [PATCH lttng-tools 6/8] Fix: tests: remove killall, add missing SIGTERM handlers Mathieu Desnoyers
@ 2014-11-19 21:40 ` Mathieu Desnoyers
  2014-11-19 21:40 ` [PATCH lttng-tools 8/8] Fix: add missing synchronization point for before app test case Mathieu Desnoyers
       [not found] ` <1416433232-20080-9-git-send-email-mathieu.desnoyers@efficios.com>
  8 siblings, 0 replies; 10+ messages in thread
From: Mathieu Desnoyers @ 2014-11-19 21:40 UTC (permalink / raw)
  To: jgalar; +Cc: lttng-dev

Also: Don't hide kill errors.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 tests/regression/tools/live/test_kernel              | 4 ++--
 tests/regression/tools/live/test_ust                 | 4 ++--
 tests/regression/tools/live/test_ust_tracefile_count | 4 ++--
 tests/regression/tools/mi/test_mi                    | 6 ++----
 tests/regression/tools/snapshots/test_ust_streaming  | 2 +-
 tests/regression/tools/snapshots/ust_test            | 2 +-
 tests/regression/ust/overlap/test_overlap            | 3 ++-
 7 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/tests/regression/tools/live/test_kernel b/tests/regression/tools/live/test_kernel
index bf5e79c..4b958df 100755
--- a/tests/regression/tools/live/test_kernel
+++ b/tests/regression/tools/live/test_kernel
@@ -88,7 +88,7 @@ clean_live_tracing
 
 # Kill the relayd
 PID_RELAYD=`pidof lt-$RELAYD_BIN`
-kill $PID_RELAYD >/dev/null 2>&1
+kill $PID_RELAYD
 if [ $? -eq 1 ]; then
 	echo "Kill lttng-relayd (pid: $PID_RELAYD)"
 	exit 1
@@ -102,7 +102,7 @@ fi
 
 # Kill the sessiond
 PID_SESSIOND=`pidof lt-$SESSIOND_BIN`
-kill $PID_SESSIOND >/dev/null 2>&1
+kill $PID_SESSIOND
 if [ $? -eq 1 ]; then
 	echo "Kill sessiond daemon"
 	exit 1
diff --git a/tests/regression/tools/live/test_ust b/tests/regression/tools/live/test_ust
index 4026bba..ae69195 100755
--- a/tests/regression/tools/live/test_ust
+++ b/tests/regression/tools/live/test_ust
@@ -88,7 +88,7 @@ clean_live_tracing
 
 # Kill the relayd
 PID_RELAYD=`pidof lt-$RELAYD_BIN`
-kill $PID_RELAYD >/dev/null 2>&1
+kill $PID_RELAYD
 if [ $? -eq 1 ]; then
 	echo "Kill lttng-relayd (pid: $PID_RELAYD)"
 	exit 1
@@ -102,7 +102,7 @@ fi
 
 # Kill the sessiond
 PID_SESSIOND=`pidof lt-$SESSIOND_BIN`
-kill $PID_SESSIOND >/dev/null 2>&1
+kill $PID_SESSIOND
 if [ $? -eq 1 ]; then
 	echo "Kill sessiond daemon"
 	exit 1
diff --git a/tests/regression/tools/live/test_ust_tracefile_count b/tests/regression/tools/live/test_ust_tracefile_count
index 3ef2ee1..68e3722 100755
--- a/tests/regression/tools/live/test_ust_tracefile_count
+++ b/tests/regression/tools/live/test_ust_tracefile_count
@@ -89,7 +89,7 @@ clean_live_tracing
 
 # Kill the relayd
 PID_RELAYD=`pidof lt-$RELAYD_BIN`
-kill $PID_RELAYD >/dev/null 2>&1
+kill $PID_RELAYD
 if [ $? -eq 1 ]; then
 	echo "Kill lttng-relayd (pid: $PID_RELAYD)"
 	exit 1
@@ -103,7 +103,7 @@ fi
 
 # Kill the sessiond
 PID_SESSIOND=`pidof lt-$SESSIOND_BIN`
-kill $PID_SESSIOND >/dev/null 2>&1
+kill $PID_SESSIOND
 if [ $? -eq 1 ]; then
 	echo "Kill sessiond daemon"
 	exit 1
diff --git a/tests/regression/tools/mi/test_mi b/tests/regression/tools/mi/test_mi
index fa46b51..e690822 100755
--- a/tests/regression/tools/mi/test_mi
+++ b/tests/regression/tools/mi/test_mi
@@ -523,9 +523,8 @@ function test_list_ust_event ()
 	test "$num" -eq "12"
 	ok $? "Mi test: $num / 12 ust event fields discovered"
 
-	#Wait for last forked process
-	wait $!
-
+	#Wait for all background processes
+	wait
 }
 
 function test_start_stop () {
@@ -603,7 +602,6 @@ function test_start_stop () {
 	#Teardown
 	OUTPUT_DEST=$DEVNULL
 	destroy_lttng_sessions
-
 }
 
 function test_snapshot () {
diff --git a/tests/regression/tools/snapshots/test_ust_streaming b/tests/regression/tools/snapshots/test_ust_streaming
index ad0c56b..55c1087 100755
--- a/tests/regression/tools/snapshots/test_ust_streaming
+++ b/tests/regression/tools/snapshots/test_ust_streaming
@@ -80,7 +80,7 @@ function stop_test_apps()
 	diag "Stopping $TESTAPP_NAME"
 	for p in ${APPS_PID}; do
 		kill ${p}
-		wait ${p} 2>&1
+		wait ${p} 2>/dev/null
 	done
 	APPS_PID=
 }
diff --git a/tests/regression/tools/snapshots/ust_test b/tests/regression/tools/snapshots/ust_test
index 69feb94..1541e6e 100755
--- a/tests/regression/tools/snapshots/ust_test
+++ b/tests/regression/tools/snapshots/ust_test
@@ -71,7 +71,7 @@ function stop_test_apps()
 	diag "Stopping $TESTAPP_NAME"
 	for p in ${APPS_PID}; do
 		kill ${p}
-		wait ${p} 2>&1
+		wait ${p} 2>/dev/null
 	done
 	APPS_PID=
 }
diff --git a/tests/regression/ust/overlap/test_overlap b/tests/regression/ust/overlap/test_overlap
index add8ff1..3a8af98 100755
--- a/tests/regression/ust/overlap/test_overlap
+++ b/tests/regression/ust/overlap/test_overlap
@@ -45,8 +45,9 @@ run_demo_app()
 	cd $CURDIR/demo
 
 	# Start test
+	diag "Running application"
 	./demo-trace >/dev/null 2>&1
-	ok $? "Start application"
+	ok $? "Application done"
 
 	cd -
 }
-- 
2.1.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH lttng-tools 8/8] Fix: add missing synchronization point for before app test case
       [not found] <1416433232-20080-1-git-send-email-mathieu.desnoyers@efficios.com>
                   ` (6 preceding siblings ...)
  2014-11-19 21:40 ` [PATCH lttng-tools 7/8] Fix: tests: wait output hide Terminate errors Mathieu Desnoyers
@ 2014-11-19 21:40 ` Mathieu Desnoyers
       [not found] ` <1416433232-20080-9-git-send-email-mathieu.desnoyers@efficios.com>
  8 siblings, 0 replies; 10+ messages in thread
From: Mathieu Desnoyers @ 2014-11-19 21:40 UTC (permalink / raw)
  To: jgalar; +Cc: lttng-dev

Fixes a race where the application could generate all its events before
trace start.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 .../regression/tools/snapshots/test_ust_streaming  |  2 +-
 tests/regression/tools/snapshots/ust_test          |  2 +-
 tests/regression/tools/streaming/test_ust          | 23 ++++++--
 .../regression/ust/before-after/test_before_after  | 11 ++--
 tests/regression/ust/buffers-pid/test_buffers_pid  | 26 ++++++---
 tests/regression/ust/java-jul/test_java_jul        | 11 +++-
 tests/regression/ust/java-log4j/test_java_log4j    | 11 +++-
 tests/regression/ust/nprocesses/test_nprocesses    | 14 +++--
 .../test_periodical_metadata_flush                 |  2 +-
 .../utils/testapp/gen-ust-events/gen-ust-events.c  | 63 +++++++++++++++++-----
 10 files changed, 128 insertions(+), 37 deletions(-)

diff --git a/tests/regression/tools/snapshots/test_ust_streaming b/tests/regression/tools/snapshots/test_ust_streaming
index 55c1087..ef38bd5 100755
--- a/tests/regression/tools/snapshots/test_ust_streaming
+++ b/tests/regression/tools/snapshots/test_ust_streaming
@@ -58,7 +58,7 @@ function snapshot_add_output ()
 # Start trace application and return once one event has been hit.
 function start_test_app()
 {
-	local tmp_file="/tmp/lttng_test_ust.42.file"
+	local tmp_file=$(mktemp -u)
 
 	# Start application with a temporary file.
 	$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT $tmp_file &
diff --git a/tests/regression/tools/snapshots/ust_test b/tests/regression/tools/snapshots/ust_test
index 1541e6e..cbc2058 100755
--- a/tests/regression/tools/snapshots/ust_test
+++ b/tests/regression/tools/snapshots/ust_test
@@ -49,7 +49,7 @@ NUM_TESTS=$(($NUM_TESTS + ($NR_SNAPSHOT * 2)))
 
 function start_test_app()
 {
-	local tmp_file="/tmp/lttng_test_ust.42.file"
+	local tmp_file=$(mktemp -u)
 
 	# Start application with a temporary file.
 	$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT $tmp_file &
diff --git a/tests/regression/tools/streaming/test_ust b/tests/regression/tools/streaming/test_ust
index 4807175..330a707 100755
--- a/tests/regression/tools/streaming/test_ust
+++ b/tests/regression/tools/streaming/test_ust
@@ -49,40 +49,53 @@ function lttng_create_session_uri
 
 function test_ust_before_start ()
 {
+	local file_sync_after_first=$(mktemp -u)
+	local file_sync_before_last=$(mktemp -u)
+
 	diag "Test UST streaming BEFORE tracing starts"
 	lttng_create_session_uri
 	enable_ust_lttng_event $SESSION_NAME $EVENT_NAME
 
 	# Run 5 times with a 1 second delay
-	$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT >/dev/null 2>&1 &
-
-	#FIXME: racy missing synchro
+	$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT ${file_sync_after_first} ${file_sync_before_last} /dev/null 2>&1 &
 
 	start_lttng_tracing $SESSION_NAME
+
+	touch ${file_sync_before_last}
+
 	# Wait for the applications started in background
 	wait
 
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
+	rm -f ${file_sync_after_first}
+	rm -f ${file_sync_before_last}
 }
 
 function test_ust_after_start ()
 {
+	local file_sync_after_first=$(mktemp -u)
+	local file_sync_before_last=$(mktemp -u)
+
 	diag "Test UST streaming AFTER tracing starts"
 	lttng_create_session_uri
 	enable_ust_lttng_event $SESSION_NAME $EVENT_NAME
 	start_lttng_tracing $SESSION_NAME
 
 	# Run 5 times with a 1 second delay
-	$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT >/dev/null 2>&1
+	$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT ${file_sync_after_first} ${file_sync_before_last} >/dev/null 2>&1
 
-	#FIXME: racy missing synchro
+	while [ ! -f "${file_sync_after_first}" ]; do
+		sleep 0.5
+	done
 
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
 
 	# Wait for the applications started in background
 	wait
+	rm -f ${file_sync_after_first}
+	rm -f ${file_sync_before_last}
 }
 
 plan_tests $NUM_TESTS
diff --git a/tests/regression/ust/before-after/test_before_after b/tests/regression/ust/before-after/test_before_after
index 1535f36..460b04c 100755
--- a/tests/regression/ust/before-after/test_before_after
+++ b/tests/regression/ust/before-after/test_before_after
@@ -57,22 +57,27 @@ function test_before_apps()
 function test_after_apps()
 {
 	local out
+	local file_sync_after_first=$(mktemp -u)
+	local file_sync_before_last=$(mktemp -u)
 
 	create_lttng_session $SESSION_NAME $TRACE_PATH
 	enable_ust_lttng_event $SESSION_NAME $EVENT_NAME
 
-	$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT &
+	$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT ${file_sync_after_first} ${file_sync_before_last} &
 	ok $? "Application started in background."
 
-	#FIXME: racy missing synchronization
-
 	start_lttng_tracing $SESSION_NAME
 
+	touch ${file_sync_before_last}
+
 	wait
 
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
 
+	rm -f ${file_sync_after_first}
+	rm -f ${file_sync_before_last}
+
 	validate_trace $EVENT_NAME $TRACE_PATH
 	out=$?
 
diff --git a/tests/regression/ust/buffers-pid/test_buffers_pid b/tests/regression/ust/buffers-pid/test_buffers_pid
index 0258e89..c95a4a1 100755
--- a/tests/regression/ust/buffers-pid/test_buffers_pid
+++ b/tests/regression/ust/buffers-pid/test_buffers_pid
@@ -74,22 +74,26 @@ test_after_multiple_apps() {
 test_before_multiple_apps() {
 	local out
 	local i
+	local file_sync_after_first=$(mktemp -u)
+	local file_sync_before_last=$(mktemp -u)
 
 	diag "Start multiple applications BEFORE tracing is started"
 
 	for i in `seq 1 5`; do
-		$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT >/dev/null 2>&1 &
+		$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT ${file_sync_after_first}_${i} ${file_sync_before_last}_${i} >/dev/null 2>&1 &
 		ok $? "Start application $i for tracing"
 	done
 
-	#FIXME: racy missing synchronization
-
 	# BEFORE application is spawned
 	create_lttng_session $SESSION_NAME $TRACE_PATH
 	enable_channel_per_pid $SESSION_NAME "channel0"
 	enable_ust_lttng_event $SESSION_NAME $EVENT_NAME "channel0"
 	start_lttng_tracing $SESSION_NAME
 
+	for i in `seq 1 5`; do
+		touch ${file_sync_before_last}_${i}
+	done
+
 	diag "Waiting for applications to end"
 	wait
 	pass "Waiting done"
@@ -97,6 +101,11 @@ test_before_multiple_apps() {
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
 
+	for i in `seq 1 5`; do
+		rm -f ${file_sync_after_first}_${i}
+		rm -f ${file_sync_before_last}_${i}
+	done
+
 	out=$(babeltrace $TRACE_PATH | grep $EVENT_NAME | wc -l)
 	if [ $out -eq 0 ]; then
 		fail "Trace validation"
@@ -136,6 +145,8 @@ test_after_app() {
 
 test_before_app() {
 	local out
+	local file_sync_after_first=$(mktemp -u)
+	local file_sync_before_last=$(mktemp -u)
 
 	diag "Start application BEFORE tracing is started"
 
@@ -145,18 +156,21 @@ test_before_app() {
 	enable_channel_per_pid $SESSION_NAME "channel0"
 	enable_ust_lttng_event $SESSION_NAME $EVENT_NAME "channel0"
 
-	$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT &
+	$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT ${file_sync_after_first} ${file_sync_before_last} &
 	ok $? "Start application to trace"
 
-	#FIXME: racy missing synchronization
-
 	start_lttng_tracing $SESSION_NAME
 
+	touch ${file_sync_before_last}
+
 	wait
 
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
 
+	rm -f ${file_sync_after_first}
+	rm -f ${file_sync_before_last}
+
 	out=$(babeltrace $TRACE_PATH | grep $EVENT_NAME | wc -l)
 	if [ $out -eq 0 ]; then
 		fail "Trace validation"
diff --git a/tests/regression/ust/java-jul/test_java_jul b/tests/regression/ust/java-jul/test_java_jul
index d16880f..d32d9ed 100755
--- a/tests/regression/ust/java-jul/test_java_jul
+++ b/tests/regression/ust/java-jul/test_java_jul
@@ -38,6 +38,7 @@ function run_app
 	local finest_tp=$1
 	local fire_second_tp=$2
 
+	#FIXME: app should have synchro.
 	java -cp $JAVA_CP -Djava.library.path="/usr/local/lib:/usr/lib" $TESTAPP_NAME $NR_ITER $NR_MSEC_WAIT $finest_tp $fire_second_tp >/dev/null 2>&1
 }
 
@@ -89,6 +90,9 @@ function enable_jul_filter_loglevel_only()
 
 function test_jul_before_start ()
 {
+	local file_sync_after_first=$(mktemp -u)
+	local file_sync_before_last=$(mktemp -u)
+
 	diag "Test JUL application BEFORE tracing starts"
 	create_lttng_session $SESSION_NAME $TRACE_PATH
 	enable_jul_lttng_event $SESSION_NAME $EVENT_NAME
@@ -96,16 +100,19 @@ function test_jul_before_start ()
 	# Run 5 times with a 1 second delay
 	run_app_background
 
-	#FIXME: racy missing synchronization
-
 	start_lttng_tracing $SESSION_NAME
 
+	touch ${file_sync_before_last}
+
 	# Wait for the applications started in background
 	wait
 
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
 
+	rm -f ${file_sync_after_first}
+	rm -f ${file_sync_before_last}
+
 	# Validate test. Expecting all events.
 	trace_match_only $EVENT_NAME $NR_ITER $TRACE_PATH
 	if [ $? -ne 0 ]; then
diff --git a/tests/regression/ust/java-log4j/test_java_log4j b/tests/regression/ust/java-log4j/test_java_log4j
index 11845a0..a0b7cdc 100755
--- a/tests/regression/ust/java-log4j/test_java_log4j
+++ b/tests/regression/ust/java-log4j/test_java_log4j
@@ -39,6 +39,7 @@ function run_app
 	local debug_tp=$1
 	local fire_second_tp=$2
 
+	# FIXME: test app should have synchro.
 	java -cp $JAVA_CP -Djava.library.path="/usr/local/lib:/usr/lib" $TESTAPP_NAME $NR_ITER $NR_MSEC_WAIT $debug_tp $fire_second_tp >/dev/null 2>&1
 }
 
@@ -90,6 +91,9 @@ function enable_log4j_filter_loglevel_only()
 
 function test_log4j_before_start ()
 {
+	local file_sync_after_first=$(mktemp -u)
+	local file_sync_before_last=$(mktemp -u)
+
 	diag "Test LOG4J application BEFORE tracing starts"
 	create_lttng_session $SESSION_NAME $TRACE_PATH
 	enable_log4j_lttng_event $SESSION_NAME $EVENT_NAME
@@ -97,16 +101,19 @@ function test_log4j_before_start ()
 	# Run 5 times with a 1 second delay
 	run_app_background
 
-	#FIXME: racy missing synchronization
-
 	start_lttng_tracing $SESSION_NAME
 
+	touch ${file_sync_before_last}
+
 	# Wait for the applications started in background
 	wait
 
 	stop_lttng_tracing $SESSION_NAME
 	destroy_lttng_session $SESSION_NAME
 
+	rm -f ${file_sync_after_first}
+	rm -f ${file_sync_before_last}
+
 	# Validate test. Expecting all events.
 	trace_match_only $EVENT_NAME $NR_ITER $TRACE_PATH
 	if [ $? -ne 0 ]; then
diff --git a/tests/regression/ust/nprocesses/test_nprocesses b/tests/regression/ust/nprocesses/test_nprocesses
index ca0f9d4..8eed95f 100755
--- a/tests/regression/ust/nprocesses/test_nprocesses
+++ b/tests/regression/ust/nprocesses/test_nprocesses
@@ -46,15 +46,16 @@ start_lttng_sessiond
 
 # Start tests. Each is an infinite tracing loop.
 
+file_sync_after_first=$(mktemp -u)
+file_sync_before_last=$(mktemp -u)
+
 diag "Starting $NUM_PROCESS test applications"
 for i in `seq 1 $NUM_PROCESS`
 do
-	$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT >/dev/null 2>&1 &
+	$TESTAPP_BIN $NR_ITER $NR_USEC_WAIT ${file_sync_after_first} ${file_sync_before_last} >/dev/null 2>&1 &
 	APPS_PID="${APPS_PID} ${!}"
 done
 
-#FIXME: racy missing synchronization
-
 diag "Waiting for applications to be registered to sessiond"
 
 reg_app_count=0
@@ -72,13 +73,15 @@ create_lttng_session $SESSION_NAME $TRACE_PATH
 enable_ust_lttng_event $SESSION_NAME $EVENT_NAME
 start_lttng_tracing $SESSION_NAME
 
+touch ${file_sync_before_last}
+
 # We don't validate whether the applications have traced here, rather
 # just that they registered to sessiond (above).
 
 stop_lttng_tracing $SESSION_NAME
 destroy_lttng_session $SESSION_NAME
 
-#FIXME/TODO: add validation after fixing racy synchroniaation
+#TODO: add trace validation.
 
 rm -rf $TRACE_PATH
 
@@ -90,4 +93,7 @@ done
 APPS_PID=
 pass "Stopped all spawned applications"
 
+rm -f ${file_sync_after_first}
+rm -f ${file_sync_before_last}
+
 stop_lttng_sessiond
diff --git a/tests/regression/ust/periodical-metadata-flush/test_periodical_metadata_flush b/tests/regression/ust/periodical-metadata-flush/test_periodical_metadata_flush
index 53db813..7937fa2 100755
--- a/tests/regression/ust/periodical-metadata-flush/test_periodical_metadata_flush
+++ b/tests/regression/ust/periodical-metadata-flush/test_periodical_metadata_flush
@@ -27,7 +27,7 @@ SESSION_NAME="periodical-flush"
 EVENT_NAME="tp:tptest"
 BIN_NAME="gen-nevents"
 NUM_TESTS=38
-APP_TMP_FILE="/tmp/lttng_test_ust.42.file"
+APP_TMP_FILE=$(mktemp -u)
 APPS_PID=
 
 source $TESTDIR/utils/utils.sh
diff --git a/tests/utils/testapp/gen-ust-events/gen-ust-events.c b/tests/utils/testapp/gen-ust-events/gen-ust-events.c
index 42fa082..60f1c6f 100644
--- a/tests/utils/testapp/gen-ust-events/gen-ust-events.c
+++ b/tests/utils/testapp/gen-ust-events/gen-ust-events.c
@@ -15,6 +15,7 @@
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
  */
 
+#define _GNU_SOURCE
 #include <assert.h>
 #include <arpa/inet.h>
 #include <fcntl.h>
@@ -27,15 +28,21 @@
 #include <sys/types.h>
 #include <unistd.h>
 #include <stdbool.h>
+#include <signal.h>
+#include <poll.h>
+#include <errno.h>
 
 #define TRACEPOINT_DEFINE
 #include "tp.h"
 
 void create_file(const char *path)
 {
+	static bool file_created = false;
 	int ret;
 
-	assert(path);
+	if (!path || file_created) {
+		return;
+	}
 
 	ret = creat(path, S_IRWXU);
 	if (ret < 0) {
@@ -44,6 +51,30 @@ void create_file(const char *path)
 	}
 
 	(void) close(ret);
+	file_created = true;
+}
+
+static
+void wait_on_file(const char *path)
+{
+	if (!path) {
+		return;
+	}
+	for (;;) {
+		int ret;
+		struct stat buf;
+
+		ret = stat(path, &buf);
+		if (ret == -1 && errno == ENOENT) {
+			(void) poll(NULL, 0, 10);	/* 10 ms delay */
+			continue;			/* retry */
+		}
+		if (ret) {
+			perror("stat");
+			exit(EXIT_FAILURE);
+		}
+		break;	/* found */
+	}
 }
 
 int main(int argc, char **argv)
@@ -55,8 +86,8 @@ int main(int argc, char **argv)
 	float flt = 2222.0;
 	int nr_iter = 100;
 	useconds_t nr_usec = 0;
-	char *tmp_file_path = NULL;
-	bool file_created = false;
+	char *after_first_event_file_path = NULL;
+	char *before_last_event_file_path = NULL;
 
 	if (argc >= 2) {
 		/*
@@ -71,22 +102,30 @@ int main(int argc, char **argv)
 	}
 
 	if (argc >= 4) {
-		tmp_file_path = argv[3];
+		after_first_event_file_path = argv[3];
+	}
+
+	if (argc >= 5) {
+		before_last_event_file_path = argv[4];
 	}
 
 	for (i = 0; nr_iter < 0 || i < nr_iter; i++) {
+		if (nr_iter >= 0 && i == nr_iter) {
+			/*
+			 * Wait on synchronization before writing last
+			 * event.
+			 */
+			wait_on_file(before_last_event_file_path);
+		}
 		netint = htonl(i);
-		tracepoint(tp, tptest, i, netint, values, text, strlen(text), dbl,
-				flt);
+		tracepoint(tp, tptest, i, netint, values, text,
+			strlen(text), dbl, flt);
 
 		/*
-		 * First loop we create the file if asked to indicate that at least one
-		 * tracepoint has been hit.
+		 * First loop we create the file if asked to indicate
+		 * that at least one tracepoint has been hit.
 		 */
-		if (!file_created && tmp_file_path) {
-			create_file(tmp_file_path);
-			file_created = true;
-		}
+		create_file(after_first_event_file_path);
 		usleep(nr_usec);
 	}
 
-- 
2.1.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH lttng-tools 8/8] Fix: add missing synchronization point for before app test case
       [not found] ` <1416433232-20080-9-git-send-email-mathieu.desnoyers@efficios.com>
@ 2014-11-19 22:45   ` Mathieu Desnoyers
       [not found]   ` <187608591.16819.1416437159675.JavaMail.zimbra@efficios.com>
  1 sibling, 0 replies; 10+ messages in thread
From: Mathieu Desnoyers @ 2014-11-19 22:45 UTC (permalink / raw)
  To: jgalar; +Cc: lttng-dev

----- Original Message -----
> From: "Mathieu Desnoyers" <mathieu.desnoyers@efficios.com>
> To: jgalar@efficios.com
> Cc: lttng-dev@lists.lttng.org, "Mathieu Desnoyers" <mathieu.desnoyers@efficios.com>
> Sent: Wednesday, November 19, 2014 10:40:32 PM
> Subject: [PATCH lttng-tools 8/8] Fix: add missing synchronization point for before app test case
> 
> Fixes a race where the application could generate all its events before
> trace start.
> 
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> ---
[...]
> b/tests/utils/testapp/gen-ust-events/gen-ust-events.c
> index 42fa082..60f1c6f 100644
> --- a/tests/utils/testapp/gen-ust-events/gen-ust-events.c
> +++ b/tests/utils/testapp/gen-ust-events/gen-ust-events.c
> @@ -15,6 +15,7 @@
>   * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
>   */
>  
> +#define _GNU_SOURCE
>  #include <assert.h>
>  #include <arpa/inet.h>
>  #include <fcntl.h>
> @@ -27,15 +28,21 @@
>  #include <sys/types.h>
>  #include <unistd.h>
>  #include <stdbool.h>
> +#include <signal.h>
> +#include <poll.h>
> +#include <errno.h>
>  
>  #define TRACEPOINT_DEFINE
>  #include "tp.h"
>  
>  void create_file(const char *path)
>  {
> +	static bool file_created = false;
>  	int ret;
>  
> -	assert(path);
> +	if (!path || file_created) {
> +		return;
> +	}
>  
>  	ret = creat(path, S_IRWXU);
>  	if (ret < 0) {
> @@ -44,6 +51,30 @@ void create_file(const char *path)
>  	}
>  
>  	(void) close(ret);
> +	file_created = true;
> +}
> +
> +static
> +void wait_on_file(const char *path)
> +{
> +	if (!path) {
> +		return;
> +	}
> +	for (;;) {
> +		int ret;
> +		struct stat buf;
> +
> +		ret = stat(path, &buf);
> +		if (ret == -1 && errno == ENOENT) {
> +			(void) poll(NULL, 0, 10);	/* 10 ms delay */
> +			continue;			/* retry */
> +		}
> +		if (ret) {
> +			perror("stat");
> +			exit(EXIT_FAILURE);
> +		}
> +		break;	/* found */
> +	}
>  }
>  
>  int main(int argc, char **argv)
> @@ -55,8 +86,8 @@ int main(int argc, char **argv)
>  	float flt = 2222.0;
>  	int nr_iter = 100;
>  	useconds_t nr_usec = 0;
> -	char *tmp_file_path = NULL;
> -	bool file_created = false;
> +	char *after_first_event_file_path = NULL;
> +	char *before_last_event_file_path = NULL;
>  
>  	if (argc >= 2) {
>  		/*
> @@ -71,22 +102,30 @@ int main(int argc, char **argv)
>  	}
>  
>  	if (argc >= 4) {
> -		tmp_file_path = argv[3];
> +		after_first_event_file_path = argv[3];
> +	}
> +
> +	if (argc >= 5) {
> +		before_last_event_file_path = argv[4];
>  	}
>  
>  	for (i = 0; nr_iter < 0 || i < nr_iter; i++) {
> +		if (nr_iter >= 0 && i == nr_iter) {

This should be if (nr_iter >= 0 && i == nr_iter - 1)

Thanks,

Mathieu

> +			/*
> +			 * Wait on synchronization before writing last
> +			 * event.
> +			 */
> +			wait_on_file(before_last_event_file_path);
> +		}
>  		netint = htonl(i);
> -		tracepoint(tp, tptest, i, netint, values, text, strlen(text), dbl,
> -				flt);
> +		tracepoint(tp, tptest, i, netint, values, text,
> +			strlen(text), dbl, flt);
>  
>  		/*
> -		 * First loop we create the file if asked to indicate that at least one
> -		 * tracepoint has been hit.
> +		 * First loop we create the file if asked to indicate
> +		 * that at least one tracepoint has been hit.
>  		 */
> -		if (!file_created && tmp_file_path) {
> -			create_file(tmp_file_path);
> -			file_created = true;
> -		}
> +		create_file(after_first_event_file_path);
>  		usleep(nr_usec);
>  	}
>  
> --
> 2.1.1
> 
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH lttng-tools 8/8] Fix: add missing synchronization point for before app test case
       [not found]   ` <187608591.16819.1416437159675.JavaMail.zimbra@efficios.com>
@ 2014-11-20  6:40     ` Mathieu Desnoyers
  0 siblings, 0 replies; 10+ messages in thread
From: Mathieu Desnoyers @ 2014-11-20  6:40 UTC (permalink / raw)
  To: jgalar; +Cc: lttng-dev

Patch 8/8 will be replaced by a v2 (upcoming).

----- Original Message -----
> From: "Mathieu Desnoyers" <mathieu.desnoyers@efficios.com>
> To: jgalar@efficios.com
> Cc: lttng-dev@lists.lttng.org
> Sent: Wednesday, November 19, 2014 11:45:59 PM
> Subject: Re: [lttng-dev] [PATCH lttng-tools 8/8] Fix: add missing synchronization point for before app test case
> 
> ----- Original Message -----
> > From: "Mathieu Desnoyers" <mathieu.desnoyers@efficios.com>
> > To: jgalar@efficios.com
> > Cc: lttng-dev@lists.lttng.org, "Mathieu Desnoyers"
> > <mathieu.desnoyers@efficios.com>
> > Sent: Wednesday, November 19, 2014 10:40:32 PM
> > Subject: [PATCH lttng-tools 8/8] Fix: add missing synchronization point for
> > before app test case
> > 
> > Fixes a race where the application could generate all its events before
> > trace start.
> > 
> > Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> > ---
> [...]
> > b/tests/utils/testapp/gen-ust-events/gen-ust-events.c
> > index 42fa082..60f1c6f 100644
> > --- a/tests/utils/testapp/gen-ust-events/gen-ust-events.c
> > +++ b/tests/utils/testapp/gen-ust-events/gen-ust-events.c
> > @@ -15,6 +15,7 @@
> >   * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
> >   */
> >  
> > +#define _GNU_SOURCE
> >  #include <assert.h>
> >  #include <arpa/inet.h>
> >  #include <fcntl.h>
> > @@ -27,15 +28,21 @@
> >  #include <sys/types.h>
> >  #include <unistd.h>
> >  #include <stdbool.h>
> > +#include <signal.h>
> > +#include <poll.h>
> > +#include <errno.h>
> >  
> >  #define TRACEPOINT_DEFINE
> >  #include "tp.h"
> >  
> >  void create_file(const char *path)
> >  {
> > +	static bool file_created = false;
> >  	int ret;
> >  
> > -	assert(path);
> > +	if (!path || file_created) {
> > +		return;
> > +	}
> >  
> >  	ret = creat(path, S_IRWXU);
> >  	if (ret < 0) {
> > @@ -44,6 +51,30 @@ void create_file(const char *path)
> >  	}
> >  
> >  	(void) close(ret);
> > +	file_created = true;
> > +}
> > +
> > +static
> > +void wait_on_file(const char *path)
> > +{
> > +	if (!path) {
> > +		return;
> > +	}
> > +	for (;;) {
> > +		int ret;
> > +		struct stat buf;
> > +
> > +		ret = stat(path, &buf);
> > +		if (ret == -1 && errno == ENOENT) {
> > +			(void) poll(NULL, 0, 10);	/* 10 ms delay */
> > +			continue;			/* retry */
> > +		}
> > +		if (ret) {
> > +			perror("stat");
> > +			exit(EXIT_FAILURE);
> > +		}
> > +		break;	/* found */
> > +	}
> >  }
> >  
> >  int main(int argc, char **argv)
> > @@ -55,8 +86,8 @@ int main(int argc, char **argv)
> >  	float flt = 2222.0;
> >  	int nr_iter = 100;
> >  	useconds_t nr_usec = 0;
> > -	char *tmp_file_path = NULL;
> > -	bool file_created = false;
> > +	char *after_first_event_file_path = NULL;
> > +	char *before_last_event_file_path = NULL;
> >  
> >  	if (argc >= 2) {
> >  		/*
> > @@ -71,22 +102,30 @@ int main(int argc, char **argv)
> >  	}
> >  
> >  	if (argc >= 4) {
> > -		tmp_file_path = argv[3];
> > +		after_first_event_file_path = argv[3];
> > +	}
> > +
> > +	if (argc >= 5) {
> > +		before_last_event_file_path = argv[4];
> >  	}
> >  
> >  	for (i = 0; nr_iter < 0 || i < nr_iter; i++) {
> > +		if (nr_iter >= 0 && i == nr_iter) {
> 
> This should be if (nr_iter >= 0 && i == nr_iter - 1)
> 
> Thanks,
> 
> Mathieu
> 
> > +			/*
> > +			 * Wait on synchronization before writing last
> > +			 * event.
> > +			 */
> > +			wait_on_file(before_last_event_file_path);
> > +		}
> >  		netint = htonl(i);
> > -		tracepoint(tp, tptest, i, netint, values, text, strlen(text), dbl,
> > -				flt);
> > +		tracepoint(tp, tptest, i, netint, values, text,
> > +			strlen(text), dbl, flt);
> >  
> >  		/*
> > -		 * First loop we create the file if asked to indicate that at least one
> > -		 * tracepoint has been hit.
> > +		 * First loop we create the file if asked to indicate
> > +		 * that at least one tracepoint has been hit.
> >  		 */
> > -		if (!file_created && tmp_file_path) {
> > -			create_file(tmp_file_path);
> > -			file_created = true;
> > -		}
> > +		create_file(after_first_event_file_path);
> >  		usleep(nr_usec);
> >  	}
> >  
> > --
> > 2.1.1
> > 
> > 
> 
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2014-11-20  6:40 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1416433232-20080-1-git-send-email-mathieu.desnoyers@efficios.com>
2014-11-19 21:40 ` [PATCH lttng-tools 1/8] Fix: tests: don't use pidof to wait for test apps Mathieu Desnoyers
2014-11-19 21:40 ` [PATCH lttng-tools 2/8] Fix: test flaky sleep and wait patterns Mathieu Desnoyers
2014-11-19 21:40 ` [PATCH lttng-tools 3/8] Document test anti-patterns Mathieu Desnoyers
2014-11-19 21:40 ` [PATCH lttng-tools 4/8] Fix: tests: add missing wait, document missing synchro Mathieu Desnoyers
2014-11-19 21:40 ` [PATCH lttng-tools 5/8] Fix: high throughput test: reset bw limit on sigterm Mathieu Desnoyers
2014-11-19 21:40 ` [PATCH lttng-tools 6/8] Fix: tests: remove killall, add missing SIGTERM handlers Mathieu Desnoyers
2014-11-19 21:40 ` [PATCH lttng-tools 7/8] Fix: tests: wait output hide Terminate errors Mathieu Desnoyers
2014-11-19 21:40 ` [PATCH lttng-tools 8/8] Fix: add missing synchronization point for before app test case Mathieu Desnoyers
     [not found] ` <1416433232-20080-9-git-send-email-mathieu.desnoyers@efficios.com>
2014-11-19 22:45   ` Mathieu Desnoyers
     [not found]   ` <187608591.16819.1416437159675.JavaMail.zimbra@efficios.com>
2014-11-20  6:40     ` Mathieu Desnoyers

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.