From 09a872ef0b4e1432329aa42fecc61f50e9baa367 Mon Sep 17 00:00:00 2001 From: Kienan Stewart Date: Thu, 8 Feb 2024 14:29:49 -0500 Subject: [PATCH] tests: test_ust_constructor: Split test_ust_constructor binary MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Observed issue ============== The single test executable gen-ust-events-constructor covers a lot of different cases in a single executable. This decreases the legibility of the test results and debuggability of the test application as many different pieces are in play. Solution ======== The test functionality covered by the executable is split into two main parts: one using a dynamically loaded shared object, and the second using a statically linked archive. Known drawbacks =============== Rather than creating a second test script, the same script is re-used to run multiple TapGenerator sequentially. This could hamper future efforts to parallelize python-based tests. Change-Id: I86d247780ce5412570eada6ebadb83a01547f2b0 Signed-off-by: Kienan Stewart Signed-off-by: Jérémie Galarneau --- .gitignore | 3 +- .../ust-constructor/test_ust_constructor.py | 165 ++++++++++-------- tests/utils/lttngtest/environment.py | 5 +- .../gen-ust-events-constructor/Makefile.am | 53 ++++-- .../gen-ust-events-constructor/main-a.cpp | 36 ++++ .../gen-ust-events-constructor/main.cpp | 20 +-- .../gen-ust-events-constructor/obj-a.cpp | 29 +++ .../gen-ust-events-constructor/obj-a.h | 23 +++ .../gen-ust-events-constructor/obj.cpp | 11 -- .../testapp/gen-ust-events-constructor/obj.h | 6 - 10 files changed, 224 insertions(+), 127 deletions(-) create mode 100644 tests/utils/testapp/gen-ust-events-constructor/main-a.cpp create mode 100644 tests/utils/testapp/gen-ust-events-constructor/obj-a.cpp create mode 100644 tests/utils/testapp/gen-ust-events-constructor/obj-a.h diff --git a/.gitignore b/.gitignore index 152c2baf8..97694519e 100644 --- a/.gitignore +++ b/.gitignore @@ -100,7 +100,8 @@ compile_commands.json /tests/regression/ust/high-throughput/gen-events /tests/regression/tools/filtering/gen-ust-events /tests/utils/testapp/gen-ust-events/gen-ust-events -/tests/utils/testapp/gen-ust-events-constructor/gen-ust-events-constructor +/tests/utils/testapp/gen-ust-events-constructor/gen-ust-events-constructor-a +/tests/utils/testapp/gen-ust-events-constructor/gen-ust-events-constructor-so /tests/utils/testapp/gen-ust-events-constructor/uses_heap /tests/utils/testapp/gen-ust-events-ns/gen-ust-events-ns /tests/regression/tools/health/health_check diff --git a/tests/regression/ust/ust-constructor/test_ust_constructor.py b/tests/regression/ust/ust-constructor/test_ust_constructor.py index 8b1fc613f..5446fb3ec 100755 --- a/tests/regression/ust/ust-constructor/test_ust_constructor.py +++ b/tests/regression/ust/ust-constructor/test_ust_constructor.py @@ -5,6 +5,7 @@ # # SPDX-License-Identifier: GPL-2.0-only +import copy import pathlib import sys import os @@ -46,25 +47,7 @@ process.wait() if process.returncode == 0: compound_literal_on_heap = True -expected_events = [ - {"name": "tp_so_c:constructor_c_provider_shared_library", "msg": None, "count": 0}, - { - "name": "tp_a:constructor_c_provider_static_archive", - "msg": None, - "count": 0, - "may_fail": compound_literal_on_heap, - }, - { - "name": "tp_so:constructor_cplusplus_provider_shared_library", - "msg": "global - shared library define and provider", - "count": 0, - }, - { - "name": "tp_a_c:constructor_cplusplus_provider_static_archive", - "msg": "global - static archive define and provider", - "count": 0, - "may_fail": compound_literal_on_heap, - }, +expected_events_common = [ { "name": "tp:constructor_c_across_units_before_define", "msg": None, @@ -143,28 +126,8 @@ expected_events = [ "count": 0, }, {"name": "tp:constructor_cplusplus", "msg": "main() local", "count": 0}, - { - "name": "tp_so:constructor_cplusplus_provider_shared_library", - "msg": "main() local - shared library define and provider", - "count": 0, - }, - { - "name": "tp_a:constructor_cplusplus_provider_static_archive", - "msg": "main() local - static archive define and provider", - "count": 0, - }, - {"name": "tp:main", "msg": None, "count": 0}, - { - "name": "tp_a:destructor_cplusplus_provider_static_archive", - "msg": "main() local - static archive define and provider", - "count": 0, - }, - { - "name": "tp_so:destructor_cplusplus_provider_shared_library", - "msg": "main() local - shared library define and provider", - "count": 0, - }, {"name": "tp:destructor_cplusplus", "msg": "main() local", "count": 0}, + {"name": "tp:main", "msg": None, "count": 0}, { "name": "tp:destructor_cplusplus", "msg": "global - across units after provider", @@ -205,17 +168,6 @@ expected_events = [ "count": 0, "may_fail": compound_literal_on_heap, }, - { - "name": "tp_a:destructor_cplusplus_provider_static_archive", - "msg": "global - static archive define and provider", - "count": 0, - "may_fail": compound_literal_on_heap, - }, - { - "name": "tp_so:destructor_cplusplus_provider_shared_library", - "msg": "global - shared library define and provider", - "count": 0, - }, { "name": "tp:destructor_c_across_units_after_provider", "msg": None, @@ -258,23 +210,62 @@ expected_events = [ "count": 0, "may_fail": compound_literal_on_heap, }, +] +expected_events_tp_so = [ + {"name": "tp_so_c:constructor_c_provider_shared_library", "msg": None, "count": 0}, { - "name": "tp_a_c:destructor_c_provider_static_archive", - "msg": None, + "name": "tp_so:constructor_cplusplus_provider_shared_library", + "msg": "global - shared library define and provider", + "count": 0, + }, + { + "name": "tp_so:constructor_cplusplus_provider_shared_library", + "msg": "main() local - shared library define and provider", + "count": 0, + }, + { + "name": "tp_so:destructor_cplusplus_provider_shared_library", + "msg": "main() local - shared library define and provider", + "count": 0, + }, + { + "name": "tp_so:destructor_cplusplus_provider_shared_library", + "msg": "global - shared library define and provider", "count": 0, - "may_fail": compound_literal_on_heap, }, {"name": "tp_so_c:destructor_c_provider_shared_library", "msg": None, "count": 0}, ] - -num_tests = 7 + len(expected_events) +expected_events_tp_a = [ + {"name": "tp_a_c:constructor_c_provider_static_archive", "msg": None, "count": 0}, + { + "name": "tp_a:constructor_cplusplus_provider_static_archive", + "msg": "global - static archive define and provider", + "count": 0, + "may_fail": compound_literal_on_heap, + }, + { + "name": "tp_a:constructor_cplusplus_provider_static_archive", + "msg": "main() local - static archive define and provider", + "count": 0, + }, + { + "name": "tp_a:destructor_cplusplus_provider_static_archive", + "msg": "main() local - static archive define and provider", + "count": 0, + }, + { + "name": "tp_a:destructor_cplusplus_provider_static_archive", + "msg": "global - static archive define and provider", + "count": 0, + "may_fail": compound_literal_on_heap, + }, + {"name": "tp_a_c:destructor_c_provider_static_archive", "msg": None, "count": 0}, +] -def capture_trace(tap, test_env): +def capture_trace(tap, test_env, application, description): # type: (lttngtest.TapGenerator, lttngtest._Environment) -> lttngtest.LocalSessionOutputLocation - tap.diagnostic( - "Capture trace from application with instrumented C/C++ constructors/destructors" - ) + tap.diagnostic(description) session_output_location = lttngtest.LocalSessionOutputLocation( test_env.create_temporary_directory("trace") @@ -300,8 +291,10 @@ def capture_trace(tap, test_env): ) as test_case: session.start() - test_app = test_env.launch_trace_test_constructor_application() - with tap.case("Run test app".format(session_name=session.name)) as test_case: + test_app = test_env.launch_test_application(application) + with tap.case( + "Run test app '{}'".format(application, session_name=session.name) + ) as test_case: test_app.wait_for_exit() with tap.case( @@ -317,7 +310,7 @@ def capture_trace(tap, test_env): return session_output_location -def validate_trace(trace_location, tap): +def validate_trace(trace_location, tap, expected_events): # type: (pathlib.Path, lttngtest.TapGenerator) unknown_event_count = 0 @@ -366,11 +359,45 @@ def validate_trace(trace_location, tap): tap.test(unknown_event_count == 0, "Found no unexpected events") -tap = lttngtest.TapGenerator(num_tests) -tap.diagnostic("Test user space constructor/destructor instrumentation coverage") +success = True +tests = [ + { + "description": "Test user space constructor/destructor instrumentation coverage (C++ w/ static archive)", + "application": "gen-ust-events-constructor/gen-ust-events-constructor-a", + "expected_events": copy.deepcopy(expected_events_common + expected_events_tp_a), + "skip_if_application_not_present": False, + }, + { + "description": "Test user space constructor/destructor instrumentation coverage (C++ w/ dynamic object", + "application": "gen-ust-events-constructor/gen-ust-events-constructor-so", + "expected_events": copy.deepcopy( + expected_events_common + expected_events_tp_so + ), + # This application is not be built when `NO_SHARED` is set in the + # configuration options. + "skip_if_application_not_present": True, + }, +] + +success = True +for test in tests: + tap = lttngtest.TapGenerator(7 + len(test["expected_events"])) + with lttngtest.test_environment(with_sessiond=True, log=tap.diagnostic) as test_env: + try: + outputlocation = capture_trace( + tap, test_env, test["application"], test["description"] + ) + except FileNotFoundError as fne: + tap.diagnostic(fne) + if test["skip_if_application_not_present"]: + tap.skip( + "Test application '{}' not found".format(test["application"]), + tap.remaining_test_cases, + ) + break + # Warning: validate_trace mutates test['expected_events'] + validate_trace(outputlocation.path, tap, test["expected_events"]) + success = success and tap.is_successful -with lttngtest.test_environment(with_sessiond=True, log=tap.diagnostic) as test_env: - outputlocation = capture_trace(tap, test_env) - validate_trace(outputlocation.path, tap) -sys.exit(0 if tap.is_successful else 1) +sys.exit(0 if success else 1) diff --git a/tests/utils/lttngtest/environment.py b/tests/utils/lttngtest/environment.py index f0e894a69..74dcfb8d2 100644 --- a/tests/utils/lttngtest/environment.py +++ b/tests/utils/lttngtest/environment.py @@ -612,7 +612,7 @@ class _Environment(logger._Logger): wait_before_exit_file_path, ) - def launch_trace_test_constructor_application(self): + def launch_test_application(self, subpath): # type () -> TraceTestApplication """ Launch an application that will trace from within constructors. @@ -622,8 +622,7 @@ class _Environment(logger._Logger): / "tests" / "utils" / "testapp" - / "gen-ust-events-constructor" - / "gen-ust-events-constructor", + / subpath, self, ) diff --git a/tests/utils/testapp/gen-ust-events-constructor/Makefile.am b/tests/utils/testapp/gen-ust-events-constructor/Makefile.am index 5e9fd90f0..b68602b34 100644 --- a/tests/utils/testapp/gen-ust-events-constructor/Makefile.am +++ b/tests/utils/testapp/gen-ust-events-constructor/Makefile.am @@ -12,6 +12,11 @@ WARN_FLAGS = \ AM_CFLAGS += $(WARN_FLAGS) AM_CXXFLAGS += $(WARN_FLAGS) +noinst_LTLIBRARIES = libtp-a-provider.la libtp-a-define.la \ + libtp-a_c-provider.la libtp-a_c-define.la +noinst_PROGRAMS = gen-ust-events-constructor-a \ + uses_heap + if NO_SHARED # Build the shared library as a static archive if shared libraries # are disabled. @@ -21,12 +26,12 @@ else # only built static by default FORCE_SHARED_LIB_OPTIONS = -module -shared -avoid-version \ -rpath $(abs_builddir) + +noinst_LTLIBRARIES += libtp-so-provider.la libtp-so-define.la \ + libtp-so_c-provider.la libtp-so_c-define.la +noinst_PROGRAMS += gen-ust-events-constructor-so endif -noinst_LTLIBRARIES = libtp-so-provider.la libtp-so-define.la \ - libtp-so_c-provider.la libtp-so_c-define.la \ - libtp-a-provider.la libtp-a-define.la \ - libtp-a_c-provider.la libtp-a_c-define.la # dynamic libraries libtp_so_provider_la_SOURCES = \ @@ -69,9 +74,7 @@ libtp_a_c_define_la_SOURCES = \ tp-a_c.h -noinst_PROGRAMS = gen-ust-events-constructor \ - uses_heap -gen_ust_events_constructor_SOURCES = main.cpp \ +gen_ust_events_constructor_so_SOURCES = main.cpp \ 01-tp-before-define.cpp \ 02-define-tp.cpp \ 03-tp-after-define.cpp \ @@ -80,17 +83,31 @@ gen_ust_events_constructor_SOURCES = main.cpp \ obj.cpp \ obj.h \ tp.h -gen_ust_events_constructor_LDADD = $(UST_LIBS) \ - $(builddir)/libtp-so-define.la \ - $(builddir)/libtp-so-provider.la \ - $(builddir)/libtp-so_c-define.la \ - $(builddir)/libtp-so_c-provider.la \ - $(builddir)/libtp-a-define.la \ - $(builddir)/libtp-a-provider.la \ - $(builddir)/libtp-a_c-define.la \ - $(builddir)/libtp-a_c-provider.la \ - $(top_builddir)/tests/utils/libtestutils.la \ - $(DL_LIBS) +gen_ust_events_constructor_so_LDADD = $(UST_LIBS) \ + $(builddir)/libtp-so-define.la \ + $(builddir)/libtp-so-provider.la \ + $(builddir)/libtp-so_c-define.la \ + $(builddir)/libtp-so_c-provider.la \ + $(top_builddir)/tests/utils/libtestutils.la \ + $(DL_LIBS) + +gen_ust_events_constructor_a_SOURCES = main-a.cpp \ + 01-tp-before-define.cpp \ + 02-define-tp.cpp \ + 03-tp-after-define.cpp \ + 04-tp-provider.cpp \ + 05-tp-after-provider.cpp \ + obj-a.cpp \ + obj-a.h \ + tp.h +gen_ust_events_constructor_a_LDADD = $(UST_LIBS) \ + $(builddir)/libtp-a-define.la \ + $(builddir)/libtp-a-provider.la \ + $(builddir)/libtp-a_c-define.la \ + $(builddir)/libtp-a_c-provider.la \ + $(top_builddir)/tests/utils/libtestutils.la \ + $(DL_LIBS) uses_heap_SOURCES = uses_heap.cpp + endif diff --git a/tests/utils/testapp/gen-ust-events-constructor/main-a.cpp b/tests/utils/testapp/gen-ust-events-constructor/main-a.cpp new file mode 100644 index 000000000..118ce66bd --- /dev/null +++ b/tests/utils/testapp/gen-ust-events-constructor/main-a.cpp @@ -0,0 +1,36 @@ +/* + * Copyright (C) 2024 Kienan Stewart + * + * SPDX-License-Identifier: LPGL-2.1-only + */ + +#include "obj-a.h" +#include "tp-a.h" +extern "C" { +#include "tp-a_c.h" +} +#include "tp.h" + +/* Use tracepoints defined and provided by static archive. */ +void test_constructor_a(void) __attribute__((constructor)); +void test_constructor_a(void) +{ + tracepoint(tp_a_c, constructor_c_provider_static_archive); +} + +void test_destructor_a(void) __attribute__((destructor)); +void test_destructor_a(void) +{ + tracepoint(tp_a_c, destructor_c_provider_static_archive); +} + +Obja g_obja_static_archive("global - static archive define and provider"); + +int main(void) +{ + Obj l_obj("main() local"); + Obja l_obja("main() local - static archive define and provider"); + + tracepoint(tp, main); + return 0; +} diff --git a/tests/utils/testapp/gen-ust-events-constructor/main.cpp b/tests/utils/testapp/gen-ust-events-constructor/main.cpp index 8f80fb092..35240ac14 100644 --- a/tests/utils/testapp/gen-ust-events-constructor/main.cpp +++ b/tests/utils/testapp/gen-ust-events-constructor/main.cpp @@ -5,10 +5,8 @@ */ #include "obj.h" -#include "tp-a.h" -#include "tp-a_c.h" -#include "tp-so_c.h" #include "tp-so.h" +#include "tp-so_c.h" #include "tp.h" /* Use tracepoints defined and provided by shared libraries. */ @@ -26,26 +24,10 @@ void test_destructor_so(void) Objso g_objso_shared_library("global - shared library define and provider"); -/* Use tracepoints defined and provided by static archive. */ -void test_constructor_a(void) __attribute__((constructor)); -void test_constructor_a(void) -{ - tracepoint(tp_a_c, constructor_c_provider_static_archive); -} - -void test_destructor_a(void) __attribute__((destructor)); -void test_destructor_a(void) -{ - tracepoint(tp_a_c, destructor_c_provider_static_archive); -} - -Obja g_obja_static_archive("global - static archive define and provider"); - int main(void) { Obj l_obj("main() local"); Objso l_objso("main() local - shared library define and provider"); - Obja l_obja("main() local - static archive define and provider"); tracepoint(tp, main); return 0; diff --git a/tests/utils/testapp/gen-ust-events-constructor/obj-a.cpp b/tests/utils/testapp/gen-ust-events-constructor/obj-a.cpp new file mode 100644 index 000000000..58c5c94ac --- /dev/null +++ b/tests/utils/testapp/gen-ust-events-constructor/obj-a.cpp @@ -0,0 +1,29 @@ +/* + * Copyright (C) 2024 Kienan Stewart + * + * SPDX-License-Identifer: LGPL-2.1-only + */ + +#include "obj-a.h" +#include "tp-a.h" +#include "tp.h" + +Obj::Obj(const char *_msg) : msg(_msg) +{ + tracepoint(tp, constructor_cplusplus, msg); +} + +Obj::~Obj() +{ + tracepoint(tp, destructor_cplusplus, msg); +} + +Obja::Obja(const char *_msg) : msg(_msg) +{ + tracepoint(tp_a, constructor_cplusplus_provider_static_archive, msg); +} + +Obja::~Obja() +{ + tracepoint(tp_a, destructor_cplusplus_provider_static_archive, msg); +} diff --git a/tests/utils/testapp/gen-ust-events-constructor/obj-a.h b/tests/utils/testapp/gen-ust-events-constructor/obj-a.h new file mode 100644 index 000000000..cd341d8c7 --- /dev/null +++ b/tests/utils/testapp/gen-ust-events-constructor/obj-a.h @@ -0,0 +1,23 @@ +/* + * Copyright (C) 2024 Kienan Stewart + * + * SPDX-License-Identifier: LGPL-2.1-only + * + */ + +#ifndef _OBJ_A_H +#define _OBJ_A_H + +struct Obj { + const char *msg; + Obj(const char *msg); + ~Obj(); +}; + +struct Obja { + const char *msg; + Obja(const char *msg); + ~Obja(); +}; + +#endif /* _OBJ_A_H */ diff --git a/tests/utils/testapp/gen-ust-events-constructor/obj.cpp b/tests/utils/testapp/gen-ust-events-constructor/obj.cpp index 948420165..430cfe334 100644 --- a/tests/utils/testapp/gen-ust-events-constructor/obj.cpp +++ b/tests/utils/testapp/gen-ust-events-constructor/obj.cpp @@ -6,7 +6,6 @@ */ #include "obj.h" -#include "tp-a.h" #include "tp-so.h" #include "tp.h" @@ -29,13 +28,3 @@ Objso::~Objso() { tracepoint(tp_so, destructor_cplusplus_provider_shared_library, msg); } - -Obja::Obja(const char *_msg) : msg(_msg) -{ - tracepoint(tp_a, constructor_cplusplus_provider_static_archive, msg); -} - -Obja::~Obja() -{ - tracepoint(tp_a, destructor_cplusplus_provider_static_archive, msg); -} diff --git a/tests/utils/testapp/gen-ust-events-constructor/obj.h b/tests/utils/testapp/gen-ust-events-constructor/obj.h index aea1468c6..0efe5ab4f 100644 --- a/tests/utils/testapp/gen-ust-events-constructor/obj.h +++ b/tests/utils/testapp/gen-ust-events-constructor/obj.h @@ -20,10 +20,4 @@ struct Objso { ~Objso(); }; -struct Obja { - const char *msg; - Obja(const char *msg); - ~Obja(); -}; - #endif /* _OBJ_H */ -- 2.34.1