-
Notifications
You must be signed in to change notification settings - Fork 134
[Tests] Add thunder_test_support static library for plugin integration testing #2084
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 2 commits
47f1530
7840e23
8e00498
eb858a8
4d5ebda
72d7c40
fe8854f
a146f76
2e76a98
552ce60
24fae29
c532dbb
61cfb19
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
bramoosterhuis marked this conversation as resolved.
|
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,105 @@ | ||||||||||
| # ============================================================================ | ||||||||||
| # thunder_test_support - Static library for Thunder plugin integration testing | ||||||||||
| # | ||||||||||
| # This library embeds the Thunder PluginHost server (PluginServer, Controller, | ||||||||||
| # SystemInfo, etc.) into a static archive so that GTest-based test binaries | ||||||||||
| # can spin up a real Thunder runtime in-process without launching the | ||||||||||
| # standalone Thunder daemon. | ||||||||||
| # | ||||||||||
| # Usage: | ||||||||||
| # 1. Link your test executable against thunder_test_support. | ||||||||||
| # 2. Use ThunderTestRuntime::Initialize() to start the embedded server. | ||||||||||
| # 3. Call JSON-RPC or COM-RPC methods directly against loaded plugins. | ||||||||||
| # 4. Call ThunderTestRuntime::Shutdown() when done. | ||||||||||
| # | ||||||||||
| # NOTE: PluginHost.cpp is deliberately excluded — it contains main(). | ||||||||||
| # The test binary provides its own main() via GTest. | ||||||||||
| # ============================================================================ | ||||||||||
|
|
||||||||||
| find_package(Threads REQUIRED) | ||||||||||
|
|
||||||||||
| set(THUNDER_SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../../Source/Thunder") | ||||||||||
|
|
||||||||||
| set(THREADPOOL_COUNT "4" CACHE STRING "The number of threads in the thread pool for test runtime") | ||||||||||
|
||||||||||
| set(THREADPOOL_COUNT "4" CACHE STRING "The number of threads in the thread pool for test runtime") | |
| if(NOT DEFINED THREADPOOL_COUNT) | |
| set(THREADPOOL_COUNT "4") | |
| endif() |
Copilot
AI
Apr 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MODULE_NAME is set both via target_compile_definitions(... MODULE_NAME=ThunderTestRuntime) and again via #define MODULE_NAME ThunderTestRuntime in Tests/test_support/Module.cpp. This can trigger macro redefinition warnings and also forces the module name for all compiled Thunder server sources in this archive. Prefer defining MODULE_NAME only in the dedicated Module.cpp (and drop the compile definition here), similar to how the main Thunder target sets APPLICATION_NAME/THREADPOOL_COUNT but not MODULE_NAME.
| MODULE_NAME=ThunderTestRuntime |
Copilot
AI
Apr 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Install rules hardcode lib/include instead of using GNUInstallDirs variables (${CMAKE_INSTALL_LIBDIR}, ${CMAKE_INSTALL_INCLUDEDIR}) used elsewhere in the project. Using the standard variables improves portability across distros/multilib layouts.
Copilot
AI
Apr 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docs and this CMake target describe consumers linking against the thunder_test_support CMake target to pick up INTERFACE whole-archive link options, but the install rules here only install the .a and header—there is no target export / CMake package config for thunder_test_support, and the include dirs also lack an INSTALL_INTERFACE. If this library is intended for external consumers via find_package, it should be exported and install-interface include dirs should be set; otherwise the docs should avoid implying the imported target exists / carries link options.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| // Module definition for the thunder_test_support static library. | ||
| // MODULE_NAME is set to ThunderTestRuntime via -D in CMakeLists.txt, | ||
| // which overrides Source/Thunder/Module.h's default of "Application". | ||
| // This ensures all server sources and the MODULE_NAME_DECLARATION below | ||
| // use the same symbol, and trace output shows "ThunderTestRuntime". | ||
|
|
||
| #ifndef MODULE_NAME | ||
| #define MODULE_NAME ThunderTestRuntime | ||
| #endif | ||
|
|
||
| #include <core/core.h> | ||
|
|
||
| MODULE_NAME_DECLARATION(BUILD_REFERENCE) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thunder has a macro for static archives — #define MODULE_NAME ThunderTestRuntime
#include <core/core.h>
MODULE_NAME_ARCHIVE_DECLARATION |
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,231 @@ | ||||||
| #include "ThunderTestRuntime.h" | ||||||
|
|
||||||
| #include <PluginServer.h> | ||||||
| #include <core/core.h> | ||||||
| #include <fstream> | ||||||
| #include <sstream> | ||||||
|
|
||||||
| // ========================================================================== | ||||||
| // ThunderTestRuntime implementation | ||||||
| // | ||||||
| // Lifecycle: Initialize() -> [run tests] -> Shutdown() | ||||||
| // | ||||||
| // Initialize creates a unique /tmp/thunder_test_XXXXXX/ directory tree, | ||||||
| // writes a minimal Thunder config.json, parses it into PluginHost::Config, | ||||||
| // constructs PluginHost::Server, and calls Server::Open() which boots | ||||||
| // the controller and activates auto-start plugins. | ||||||
| // | ||||||
| // Shutdown reverses the process: Server::Close(), cleanup temp files. | ||||||
| // ========================================================================== | ||||||
|
|
||||||
| namespace Thunder { | ||||||
| namespace TestCore { | ||||||
|
|
||||||
| ThunderTestRuntime::~ThunderTestRuntime() | ||||||
| { | ||||||
| Shutdown(); | ||||||
| } | ||||||
|
|
||||||
| void ThunderTestRuntime::CreateDirectories() const | ||||||
| { | ||||||
| Core::Directory(_tempDir.c_str()).Create(); | ||||||
| Core::Directory((_tempDir + "persistent/").c_str()).Create(); | ||||||
| Core::Directory((_tempDir + "volatile/").c_str()).Create(); | ||||||
| Core::Directory((_tempDir + "data/").c_str()).Create(); | ||||||
|
|
||||||
| } | ||||||
|
|
||||||
| void ThunderTestRuntime::CleanupDirectories() const | ||||||
| { | ||||||
| if (!_tempDir.empty()) { | ||||||
| Core::Directory(_tempDir.c_str()).Destroy(); | ||||||
|
|
||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // Build a minimal Thunder JSON config from the plugin list. | ||||||
| // Uses port 0 (OS-assigned) and binds to localhost only. | ||||||
| string ThunderTestRuntime::BuildConfigJSON(const std::vector<PluginConfig>& plugins, | ||||||
| const string& systemPath, | ||||||
| const string& proxyStubPath) const | ||||||
| { | ||||||
|
bramoosterhuis marked this conversation as resolved.
|
||||||
| std::ostringstream json; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use Thunder Core JSON for JSON manipulation! |
||||||
| json << "{" | ||||||
| << "\"port\":0," | ||||||
| << "\"binding\":\"127.0.0.1\"," | ||||||
| << "\"idletime\":180," | ||||||
|
||||||
| << "\"persistentpath\":\"" << _tempDir << "persistent/\"," | ||||||
| << "\"volatilepath\":\"" << _tempDir << "volatile/\"," | ||||||
| << "\"datapath\":\"" << _tempDir << "data/\"," | ||||||
| << "\"systempath\":\"" << systemPath << "\"," | ||||||
| << "\"proxystubpath\":\"" << proxyStubPath << "\"," | ||||||
| << "\"communicator\":\"" << _tempDir << "communicator\"," | ||||||
| << "\"plugins\":["; | ||||||
|
|
||||||
| for (size_t i = 0; i < plugins.size(); ++i) { | ||||||
| const auto& p = plugins[i]; | ||||||
| if (i > 0) json << ","; | ||||||
| json << "{" | ||||||
| << "\"callsign\":\"" << p.callsign << "\"," | ||||||
| << "\"locator\":\"" << p.locator << "\"," | ||||||
| << "\"classname\":\"" << p.classname << "\"," | ||||||
| << "\"startuporder\":" << p.startuporder << "," | ||||||
| << "\"autostart\":" << (p.autostart ? "true" : "false"); | ||||||
|
||||||
| << "\"autostart\":" << (p.autostart ? "true" : "false"); | |
| << "\"startmode\":" << JsonEscape(p.autostart ? "Activated" : "Deactivated"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no autostart in 5.x in anymore. Perhaps it's then better to actually reuse the plugin Configuration as defined in plugns/Configuration.h, that would've eliminate such issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For OS-abstraction use Thunder Core functionality (also to remove posix dependency).
Copilot
AI
Apr 15, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initialize() hardcodes a POSIX /tmp/... path and uses mkdtemp(), which will not build on non-POSIX platforms (e.g., Windows) if ENABLE_TEST_RUNTIME is enabled. Either gate this feature to POSIX in CMake (with a clear message when enabled on unsupported platforms) or provide a platform-specific temp directory implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Such path must not be hardcoded! On 5.x the path is different.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use Thunder-provided functionality for normalizing paths!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is missing messaging startup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multiple return points are not as per coding standards.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ENABLE_TEST_RUNTIMEcan currently be enabled on any platform, butTests/test_support/ThunderTestRuntime.cppuses POSIX-only APIs (mkdtemp) and hardcoded/tmppaths. Consider adding a platform guard here (e.g., only add thetest_supportsubdirectory on POSIX platforms, or emit a fatal error if the option is ON on unsupported systems) to avoid configuration/build failures.