tests: test_ust_constructor: Split test_ust_constructor binary
authorKienan Stewart <kstewart@efficios.com>
Thu, 8 Feb 2024 19:29:49 +0000 (14:29 -0500)
committerJérémie Galarneau <jeremie.galarneau@efficios.com>
Thu, 7 Mar 2024 17:19:21 +0000 (12:19 -0500)
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 <kstewart@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
.gitignore
tests/regression/ust/ust-constructor/test_ust_constructor.py
tests/utils/lttngtest/environment.py
tests/utils/testapp/gen-ust-events-constructor/Makefile.am
tests/utils/testapp/gen-ust-events-constructor/main-a.cpp [new file with mode: 0644]
tests/utils/testapp/gen-ust-events-constructor/main.cpp
tests/utils/testapp/gen-ust-events-constructor/obj-a.cpp [new file with mode: 0644]
tests/utils/testapp/gen-ust-events-constructor/obj-a.h [new file with mode: 0644]
tests/utils/testapp/gen-ust-events-constructor/obj.cpp
tests/utils/testapp/gen-ust-events-constructor/obj.h

index 152c2baf89dbe0878de12310cd4e09971153f8eb..97694519e028386d4ad1f70dfb85d06665325294 100644 (file)
@@ -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
index 8b1fc613faf2dd29c6dfbbe87139ddc79e7a074b..5446fb3ec08ef1636c39cdd371e064fc13abbc3f 100755 (executable)
@@ -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)
index f0e894a69e14c470c7ac56a0f98b8fc3dab3dad4..74dcfb8d26cebf89c66fbd187d77e4eef1a0dcc6 100644 (file)
@@ -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,
         )
 
index 5e9fd90f0aeca714808e38c5877c7f59e46c0291..b68602b348a05f9f8c7fc0f2bcfc22aea5652e04 100644 (file)
@@ -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 (file)
index 0000000..118ce66
--- /dev/null
@@ -0,0 +1,36 @@
+/*
+ * Copyright (C) 2024 Kienan Stewart <kstewart@efficios.com>
+ *
+ * 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;
+}
index 8f80fb092437a76c6ffe7e2004d92071f7561fd9..35240ac146d65375d00e3a106516f7327f57c650 100644 (file)
@@ -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 (file)
index 0000000..58c5c94
--- /dev/null
@@ -0,0 +1,29 @@
+/*
+ * Copyright (C) 2024 Kienan Stewart <kstewart@efficios.com>
+ *
+ * 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 (file)
index 0000000..cd341d8
--- /dev/null
@@ -0,0 +1,23 @@
+/*
+ * Copyright (C) 2024 Kienan Stewart <kstewart@efficios.com>
+ *
+ * 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 */
index 948420165c946cf7dab445f679e3cc0a8c36b77e..430cfe3342e57a842a825a3f772f7c896e06f5d3 100644 (file)
@@ -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);
-}
index aea1468c65e61d2e8d73a677ea311386eaee5741..0efe5ab4fd47685adc0d79b3767ac550d4d651c2 100644 (file)
@@ -20,10 +20,4 @@ struct Objso {
        ~Objso();
 };
 
-struct Obja {
-       const char *msg;
-       Obja(const char *msg);
-       ~Obja();
-};
-
 #endif /* _OBJ_H */
This page took 0.033594 seconds and 4 git commands to generate.