[lttng-dev] [PATCH lttng-tools] Test for lttng-logger

[Jonathan Rajotte-Julien]

On Fri, Dec 22, 2017 at 04:22:08PM -0500, Julien Desfossez wrote:
> Basic test to write in /proc/lttng-logger and /dev/lttng-logger and
> ensure we have the right amount of events in the trace resulting trace.
> We also test the 1024 characters limit for the payload.

I feel that the current test introduced by this patch could be split in
multiple tests.

   test_dev_logger
   test_proc_logger
   test_payload_limit

"test_payload_limit" could further be split between proc and dev but it would
already be a good start even if not split.

I don't expect any of this to break anytime soon but if it does someday, having
a more granular approach will save us some time.

A bit more inline.

Cheers


> 
> Signed-off-by: Julien Desfossez <jdesfossez at efficios.com>
> ---
>  tests/regression/kernel/Makefile.am       |  2 +-
>  tests/regression/kernel/test_lttng_logger | 85 +++++++++++++++++++++++++++++++
>  tests/root_regression                     |  1 +
>  tests/utils/utils.sh                      |  2 +-
>  4 files changed, 88 insertions(+), 2 deletions(-)
>  create mode 100755 tests/regression/kernel/test_lttng_logger
> 
> diff --git a/tests/regression/kernel/Makefile.am b/tests/regression/kernel/Makefile.am
> index c4ee443..a0abc7b 100644
> --- a/tests/regression/kernel/Makefile.am
> +++ b/tests/regression/kernel/Makefile.am
> @@ -1,6 +1,6 @@
>  EXTRA_DIST = test_event_basic test_all_events test_syscall \
>  		test_clock_override test_rotation_destroy_flush \
> -		test_select_poll_epoll
> +		test_select_poll_epoll test_lttng_logger
>  
>  noinst_PROGRAMS = select_poll_epoll
>  select_poll_epoll_SOURCES = select_poll_epoll.c
> diff --git a/tests/regression/kernel/test_lttng_logger b/tests/regression/kernel/test_lttng_logger
> new file mode 100755
> index 0000000..54fd1c3
> --- /dev/null
> +++ b/tests/regression/kernel/test_lttng_logger
> @@ -0,0 +1,85 @@
> +#!/bin/bash
> +#
> +# Copyright (C) - 2017 Julien Desfossez <jdesfossez at efficios.com>
> +#
> +# This program is free software; you can redistribute it and/or modify it
> +# under the terms of the GNU General Public License, version 2 only, as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it will be useful, but WITHOUT
> +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> +# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> +# more details.
> +#
> +# You should have received a copy of the GNU General Public License along with
> +# this program; if not, write to the Free Software Foundation, Inc., 51
> +# Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> +
> +TEST_DESC="Kernel tracer - lttng-logger"
> +
> +CURDIR=$(dirname $0)/
> +TESTDIR=$CURDIR/../..
> +NUM_TESTS=13
> +
> +source $TESTDIR/utils/utils.sh
> +
> +function test_logger_normal()
> +{
> +	TRACE_PATH=$(mktemp -d)
> +	SESSION_NAME="kernel_event_basic"
> +
> +	PAYLOAD="test_logger"
> +
> +	create_lttng_session_ok $SESSION_NAME $TRACE_PATH
> +
> +	lttng_enable_kernel_event $SESSION_NAME "lttng_logger"
> +
> +	start_lttng_tracing_ok
> +
> +	test -e /proc/lttng-logger
> +	ok $? "/proc/lttng-logger exists"
> +	echo -n "$PAYLOAD proc" > /proc/lttng-logger
> +
> +	test -c /dev/lttng-logger
> +	if test $? = 0; then

Why is this handled differently than /proc/lttng-logger ?

Did you want to prevent writing to /dev/lttng-logger?

If soi,  why not do the same for /prov/lttng-logger?

> +		pass "/dev/lttng-logger is a character device"
> +		echo -n "$PAYLOAD dev" > /dev/lttng-logger
> +		ok $? "Write in /dev/lttng-logger"
> +	else
> +		fail "No /dev/lttng-logger" 

Trailing whitespace here.

> +	fi
> +
> +	# Write 100 times "test_logger", which generates 1200 characters, we expect
> +	# the tracer to write 2 events from that string because it limits the
> +	# input to 1024 strings.
> +	printf "%.s $PAYLOAD" {1..100} >/proc/lttng-logger

Missing space here ">/proc/lttng-logger"

> +
> +	stop_lttng_tracing_ok
> +
> +	validate_trace_count "lttng_logger" $TRACE_PATH 4
> +	validate_trace_only_exp "$PAYLOAD" $TRACE_PATH
> +
> +	destroy_lttng_session_ok $SESSION_NAME
> +
> +	rm -rf $TRACE_PATH
> +}
> +
> +# MUST set TESTDIR before calling those functions
> +plan_tests $NUM_TESTS
> +
> +print_test_banner "$TEST_DESC"
> +
> +if [ "$(id -u)" == "0" ]; then
> +	isroot=1
> +else
> +	isroot=0
> +fi
> +
> +skip $isroot "Root access is needed. Skipping all tests." $NUM_TESTS ||
> +{
> +	start_lttng_sessiond
> +
> +	test_logger_normal
> +
> +	stop_lttng_sessiond
> +}
> diff --git a/tests/root_regression b/tests/root_regression
> index 7639c18..f17ac97 100644
> --- a/tests/root_regression
> +++ b/tests/root_regression
> @@ -4,6 +4,7 @@ regression/kernel/test_syscall
>  regression/kernel/test_clock_override
>  regression/kernel/test_rotation_destroy_flush
>  regression/kernel/test_select_poll_epoll
> +regression/kernel/test_lttng_logger
>  regression/tools/live/test_kernel
>  regression/tools/live/test_lttng_kernel
>  regression/tools/streaming/test_high_throughput_limits
> diff --git a/tests/utils/utils.sh b/tests/utils/utils.sh
> index e8dfcda..50b5db1 100644
> --- a/tests/utils/utils.sh
> +++ b/tests/utils/utils.sh
> @@ -1480,7 +1480,7 @@ function validate_trace_only_exp()
>  	local total=$($BABELTRACE_BIN $trace_path | wc -l)
>  
>  	if [ "$count" -ne 0 ] && [ "$total" -eq "$count" ]; then
> -		pass "Trace match with $total for expression '${event_exp}"
> +		pass "Trace match with $total for expression '${event_exp}'"
>  	else
>  		fail "Trace match"
>  		diag "$total syscall event(s) found, only syscalls matching expression '${event_exp}' ($count occurrences) are expected"
> -- 
> 2.7.4
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev at lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

-- 
Jonathan Rajotte-Julien
EfficiOS