From 1773925bf775632869667ee77fca7adf201ff2df Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Mon, 27 Jul 2026 21:40:49 -0400 Subject: [PATCH] refactor: smart pointers and fix ownership Replace raw pointer members (app, trayIcon, trayTopMenu) with std::unique_ptr in QtTrayMenu. Consolidate tray_qt module-level globals into a State struct accessed via a function-local static. Move tray test fixtures from global arrays into per-fixture instance members to eliminate shared mutable state between tests. Replace std::system shell invocations with posix_spawnp for notification cleanup. Use placement new instead of reinterpret_cast+memcpy for C struct initialization over raw buffers. --- src/QtTrayMenu.cpp | 57 ++++++------------- src/QtTrayMenu.h | 13 ++++- src/tray_qt.cpp | 99 ++++++++++++++++---------------- tests/notification_utils.cpp | 48 ++++++++++++---- tests/screenshot_utils.cpp | 29 +++++----- tests/unit/test_tray.cpp | 106 ++++++++++++++--------------------- tests/unit/test_tray_qt.cpp | 73 +++++++++++++----------- 7 files changed, 211 insertions(+), 214 deletions(-) diff --git a/src/QtTrayMenu.cpp b/src/QtTrayMenu.cpp index 74d9e284..1bcf5e36 100644 --- a/src/QtTrayMenu.cpp +++ b/src/QtTrayMenu.cpp @@ -19,12 +19,6 @@ #include "WindowsAppearance.h" #endif -namespace { - int defaultArgc = 1; // NOSONAR(cpp:S5421): This is required for QApplication's argc/argv constructor - char defaultArgv0[] = "TrayMenuApp"; // NOSONAR(cpp:S5421): This is required for QApplication's argc/argv constructor - char *defaultArgv[] = {defaultArgv0, nullptr}; // NOSONAR(cpp:S5421,cpp:S5954): This is required for QApplication's argc/argv constructor -} // namespace - QtTrayMenu::QtTrayMenu(QObject *parent, const bool debug): QtTrayMenu(-1, nullptr, parent, debug) { }; @@ -40,9 +34,9 @@ QtTrayMenu::QtTrayMenu(int argc, char **argv, QObject *parent, const bool debug) // Note: The following is ugly but QApplication requires an argv containing the application name. // We might not have access to the real argc/argv here due to being called/pulled as a dependency. if (argc < 0 && argv == nullptr) { - app = new QApplication(defaultArgc, defaultArgv); // NOSONAR(cpp:S5025): Qt has its own integrated memory management + app = new QApplication(defaultArgc, defaultArgv.data()); // NOSONAR(cpp:S5025): QApplication must remain alive through process teardown } else { - app = new QApplication(argc, argv); // NOSONAR(cpp:S5025): Qt has its own integrated memory management + app = new QApplication(argc, argv); // NOSONAR(cpp:S5025): QApplication must remain alive through process teardown } } #if defined(_WIN32) @@ -53,16 +47,7 @@ QtTrayMenu::QtTrayMenu(int argc, char **argv, QObject *parent, const bool debug) } } -QtTrayMenu::~QtTrayMenu() { - // Cleanup app only if it was created within this class - if (app && app != QApplication::instance()) { - // Quit QApplication - QApplication::quit(); - // Delete app and clear references - delete app; // NOSONAR(cpp:S5025): Qt has its own integrated memory management - app = nullptr; // Set to nullptr after deletion - } -} +QtTrayMenu::~QtTrayMenu() = default; int QtTrayMenu::init(struct tray *tray, const bool notification) { if (trayIcon) { @@ -82,18 +67,18 @@ int QtTrayMenu::init(struct tray *tray, const bool notification) { } // Create tray icon - trayIcon = new QSystemTrayIcon(lookupIcon(tray->icon), this); + trayIcon = std::make_unique(lookupIcon(tray->icon)); trayIcon->setToolTip(QString::fromUtf8(tray->tooltip)); - connect(trayIcon, &QSystemTrayIcon::activated, this, &QtTrayMenu::onTrayActivated); - connect(trayIcon, &QSystemTrayIcon::messageClicked, this, &QtTrayMenu::onMessageClicked); + connect(trayIcon.get(), &QSystemTrayIcon::activated, this, &QtTrayMenu::onTrayActivated); + connect(trayIcon.get(), &QSystemTrayIcon::messageClicked, this, &QtTrayMenu::onMessageClicked); connect(this, &QtTrayMenu::update, this, &QtTrayMenu::onUpdate); connect(this, &QtTrayMenu::exit, this, &QtTrayMenu::onExitRequested); connect(this, &QtTrayMenu::showMenu, this, &QtTrayMenu::onShowMenu); updateMenu(tray->menu); - trayIcon->setContextMenu(trayTopMenu); + trayIcon->setContextMenu(trayTopMenu.get()); trayIcon->show(); if (notification) { @@ -147,14 +132,12 @@ void QtTrayMenu::onExitRequested() { if (trayIcon) { trayIcon->setContextMenu(nullptr); } - delete trayTopMenu; // NOSONAR(cpp:S5025): Qt has its own integrated memory management - trayTopMenu = nullptr; // Set to nullptr after deletion + trayTopMenu.reset(); } // Remove tray icon references; if (trayIcon) { trayIcon->hide(); - delete trayIcon; // NOSONAR(cpp:S5025): Qt has its own integrated memory management - trayIcon = nullptr; // Set to nullptr after deletion + trayIcon.reset(); } // Unset tray structure trayStruct = nullptr; @@ -167,22 +150,16 @@ void QtTrayMenu::onExitRequested() { void QtTrayMenu::updateMenu(struct tray_menu *items) { // Create and setup new tray menu instance - const auto newTrayTopMenu = new QMenu(); // NOSONAR(cpp:S5025): Qt has its own integrated memory management + auto newTrayTopMenu = std::make_unique(); #if defined(_WIN32) - connect(newTrayTopMenu, &QMenu::aboutToShow, this, []() { + connect(newTrayTopMenu.get(), &QMenu::aboutToShow, this, []() { tray_qt::windows::sync_color_scheme(); }); #endif - trayIcon->setContextMenu(newTrayTopMenu); + trayIcon->setContextMenu(newTrayTopMenu.get()); // Fill new tray menu instance - createMenu(items, newTrayTopMenu); - // Clear old, unused trayTopMenu instance - if (trayTopMenu != nullptr) { - trayTopMenu->clear(); // Remove all actions - delete trayTopMenu; // NOSONAR(cpp:S5025): Qt has its own integrated memory management - } - // Store reference for cleanup - trayTopMenu = newTrayTopMenu; + createMenu(items, newTrayTopMenu.get()); + trayTopMenu = std::move(newTrayTopMenu); } void QtTrayMenu::createMenu(struct tray_menu *items, QMenu *menu) { @@ -190,7 +167,7 @@ void QtTrayMenu::createMenu(struct tray_menu *items, QMenu *menu) { if (strcmp(items->text, "-") == 0) { menu->addSeparator(); } else { - auto *action = new QAction(QString::fromUtf8(items->text), menu); // NOSONAR(cpp:S5025): Qt has its own integrated memory management + auto *action = menu->addAction(QString::fromUtf8(items->text)); action->setDisabled(items->disabled == 1); action->setCheckable(items->checkbox == 1); action->setChecked(items->checked == 1); @@ -249,7 +226,7 @@ void QtTrayMenu::onTrayActivated(QSystemTrayIcon::ActivationReason reason) { } void QtTrayMenu::onMenuItemTriggered() { - auto *action = qobject_cast(sender()); + const auto *action = qobject_cast(sender()); struct tray_menu *menuItem = getTrayMenuItem(action); if (menuItem && menuItem->cb) { @@ -257,7 +234,7 @@ void QtTrayMenu::onMenuItemTriggered() { } } -struct tray_menu *QtTrayMenu::getTrayMenuItem(QAction *action) { // NOSONAR(cpp:S995): Use as defined in function interface +struct tray_menu *QtTrayMenu::getTrayMenuItem(const QAction *action) { return static_cast(action->property("tray_menu_item").value()); } diff --git a/src/QtTrayMenu.h b/src/QtTrayMenu.h index af5039b8..781cef6f 100644 --- a/src/QtTrayMenu.h +++ b/src/QtTrayMenu.h @@ -5,6 +5,10 @@ #ifndef TRAYMENU_H #define TRAYMENU_H +// standard includes +#include +#include + // qt includes #include #include @@ -135,13 +139,16 @@ class QtTrayMenu: public QObject { void createNotification(); void updateMenu(struct tray_menu *items); QIcon lookupIcon(QString icon) const; + int defaultArgc = 1; + std::array defaultArgv0 {'T', 'r', 'a', 'y', 'M', 'e', 'n', 'u', 'A', 'p', 'p', '\0'}; + std::array defaultArgv {defaultArgv0.data(), nullptr}; QApplication *app = nullptr; - QSystemTrayIcon *trayIcon = nullptr; - QMenu *trayTopMenu = nullptr; + std::unique_ptr trayIcon; + std::unique_ptr trayTopMenu; struct tray *trayStruct = nullptr; bool running = false; bool blockingEventLoop = false; - struct tray_menu *getTrayMenuItem(QAction *action); + struct tray_menu *getTrayMenuItem(const QAction *action); mutable std::function notificationCallback = nullptr; private slots: diff --git a/src/tray_qt.cpp b/src/tray_qt.cpp index ebe34af6..8877dbd4 100644 --- a/src/tray_qt.cpp +++ b/src/tray_qt.cpp @@ -19,36 +19,32 @@ namespace tray_qt { /** - * QtTrayMenu instance + * @brief Process-wide state backing the C tray API. */ - std::unique_ptr qt_tray_menu = nullptr; // NOSONAR(cpp:S5421): mutable state, not const - /** - * Logging callback for qt_message_handler - */ - void (*log_callback)(int, const char *) = nullptr; // NOSONAR(cpp:S5421): mutable state, not const - /** - * Explicit Qt application metadata configured through the C API. - */ - bool app_info_configured = false; // NOSONAR(cpp:S5421): mutable state, not const - /** - * Qt application name configured through the C API. - */ - QString app_name; // NOSONAR(cpp:S5421): mutable state, not const - /** - * Qt application display name configured through the C API. - */ - QString app_display_name; // NOSONAR(cpp:S5421): mutable state, not const + struct State { + std::unique_ptr trayMenu; ///< Active tray menu instance. + void (*logCallback)(int, const char *) = nullptr; ///< Registered C logging callback. + bool appInfoConfigured = false; ///< Whether application metadata was explicitly configured. + QString appName; ///< Configured application name. + QString appDisplayName; ///< Configured application display name. + QString desktopName; ///< Configured desktop file name. + }; + /** - * Qt desktop file name configured through the C API. + * @brief Access the process-wide tray API state. + * @return Mutable tray API state. */ - QString desktop_name; // NOSONAR(cpp:S5421): mutable state, not const + State &state() { + static State instance; + return instance; + } /** * @brief Acknowledge/click current notification. */ void acknowledge_notification() { - if (qt_tray_menu != nullptr && QtTrayMenu::supportsMessages()) { - qt_tray_menu->clickMessage(); + if (state().trayMenu != nullptr && QtTrayMenu::supportsMessages()) { + state().trayMenu->clickMessage(); } } @@ -56,8 +52,8 @@ namespace tray_qt { * @brief Clear current notification state without invoking callbacks. */ void clear_notification() { - if (qt_tray_menu != nullptr) { - qt_tray_menu->clearMessageCallback(); + if (state().trayMenu != nullptr) { + state().trayMenu->clearMessageCallback(); } } @@ -70,11 +66,11 @@ namespace tray_qt { clear_notification(); return; } - if (qt_tray_menu != nullptr && QtTrayMenu::supportsMessages()) { + if (state().trayMenu != nullptr && QtTrayMenu::supportsMessages()) { if (tray->notification_icon != nullptr) { - qt_tray_menu->showMessage(tray->notification_title, tray->notification_text, tray->notification_icon, tray->notification_cb); + state().trayMenu->showMessage(tray->notification_title, tray->notification_text, tray->notification_icon, tray->notification_cb); } else { - qt_tray_menu->showMessage(tray->notification_title, tray->notification_text, tray->notification_cb); + state().trayMenu->showMessage(tray->notification_title, tray->notification_text, tray->notification_cb); } } } @@ -84,14 +80,15 @@ namespace tray_qt { * @param allow_defaults Whether empty app info values should apply fallback defaults. */ void apply_app_info(const bool allow_defaults = true) { - if (!app_info_configured || qt_tray_menu == nullptr) { + const auto ¤t_state = state(); + if (!current_state.appInfoConfigured || current_state.trayMenu == nullptr) { return; } - if (!allow_defaults && app_name.isEmpty() && app_display_name.isEmpty()) { + if (!allow_defaults && current_state.appName.isEmpty() && current_state.appDisplayName.isEmpty()) { return; } - qt_tray_menu->configureAppMetadata(app_name, app_display_name, desktop_name); + current_state.trayMenu->configureAppMetadata(current_state.appName, current_state.appDisplayName, current_state.desktopName); } /** @@ -114,7 +111,7 @@ namespace tray_qt { * @param msg The message string. */ void qt_message_handler(QtMsgType type, const QMessageLogContext &, const QString &msg) { - if (log_callback == nullptr) { + if (state().logCallback == nullptr) { return; } int level; @@ -132,29 +129,31 @@ namespace tray_qt { level = 3; break; } - log_callback(level, msg.toUtf8().constData()); + state().logCallback(level, msg.toUtf8().constData()); } } // namespace tray_qt extern "C" { void tray_set_app_info(const char *app_name, const char *app_display_name, const char *desktop_name) { - tray_qt::app_info_configured = true; - tray_qt::app_name = app_name != nullptr ? QString::fromUtf8(app_name) : QString(); - tray_qt::app_display_name = app_display_name != nullptr ? QString::fromUtf8(app_display_name) : QString(); - tray_qt::desktop_name = desktop_name != nullptr ? QString::fromUtf8(desktop_name) : QString(); + auto &state = tray_qt::state(); + state.appInfoConfigured = true; + state.appName = app_name != nullptr ? QString::fromUtf8(app_name) : QString(); + state.appDisplayName = app_display_name != nullptr ? QString::fromUtf8(app_display_name) : QString(); + state.desktopName = desktop_name != nullptr ? QString::fromUtf8(desktop_name) : QString(); tray_qt::apply_app_info(); } int tray_init(struct tray *tray) { - if (tray_qt::qt_tray_menu == nullptr) { + auto &state = tray_qt::state(); + if (state.trayMenu == nullptr) { tray_qt::configure_platform(); // Create a new unique pointer to QtTrayMenu instance - tray_qt::qt_tray_menu = std::make_unique(); + state.trayMenu = std::make_unique(); tray_qt::apply_app_info(false); } - if (const auto result = tray_qt::qt_tray_menu->init(tray, false); result < 0) { + if (const auto result = state.trayMenu->init(tray, false); result < 0) { // Tray init failed. Clean up and return error. tray_exit(); return result; @@ -173,18 +172,18 @@ extern "C" { } int tray_loop(int blocking) { - if (tray_qt::qt_tray_menu == nullptr) { + if (tray_qt::state().trayMenu == nullptr) { return -1; } - return tray_qt::qt_tray_menu->loop(blocking); + return tray_qt::state().trayMenu->loop(blocking); } void tray_update(struct tray *tray) { // NOSONAR(cpp:S995): C API requires this exact mutable-pointer signature - if (tray_qt::qt_tray_menu == nullptr) { + if (tray_qt::state().trayMenu == nullptr) { return; } - auto *const tray_menu = tray_qt::qt_tray_menu.get(); + auto *const tray_menu = tray_qt::state().trayMenu.get(); const auto apply_update = [tray_menu, tray]() { tray_menu->update(tray, false); tray_qt::notify(tray); @@ -200,14 +199,14 @@ extern "C" { } void tray_exit(void) { - if (tray_qt::qt_tray_menu == nullptr) { + if (tray_qt::state().trayMenu == nullptr) { return; } - tray_qt::qt_tray_menu->exit(); + tray_qt::state().trayMenu->exit(); } void tray_set_log_callback(void (*cb)(int level, const char *msg)) { // NOSONAR(cpp:S5205): C API requires a plain function pointer callback type - tray_qt::log_callback = cb; + tray_qt::state().logCallback = cb; if (cb != nullptr) { qInstallMessageHandler(tray_qt::qt_message_handler); } else { @@ -216,17 +215,17 @@ extern "C" { } void tray_show_menu(void) { - if (tray_qt::qt_tray_menu == nullptr) { + if (tray_qt::state().trayMenu == nullptr) { return; } - tray_qt::qt_tray_menu->showMenu(); + tray_qt::state().trayMenu->showMenu(); } void tray_simulate_menu_item_click(int index) { - if (tray_qt::qt_tray_menu == nullptr) { + if (tray_qt::state().trayMenu == nullptr) { return; } - tray_qt::qt_tray_menu->clickMenuItem(index); + tray_qt::state().trayMenu->clickMenuItem(index); } void tray_simulate_notification_click(void) { diff --git a/tests/notification_utils.cpp b/tests/notification_utils.cpp index 0bc35568..5b16785c 100644 --- a/tests/notification_utils.cpp +++ b/tests/notification_utils.cpp @@ -7,26 +7,52 @@ #include "notification_utils.h" // standard includes +#include #include -#include +#include #include // lib includes #include #if defined(__linux__) + #include + #include + #include + #include + +extern char **environ; + namespace { void closeFreedesktopNotifications() { - constexpr const char *close_notifications = - "if command -v dbus-send >/dev/null 2>&1; then " - "id=1; while [ \"$id\" -le 128 ]; do " - "dbus-send --session --print-reply=literal --dest=org.freedesktop.Notifications " - "/org/freedesktop/Notifications org.freedesktop.Notifications.CloseNotification uint32:$id " - ">/dev/null 2>&1; " - "id=$((id + 1)); " - "done; " - "fi"; - (void) std::system(close_notifications); // NOSONAR(cpp:S4721): test-only cleanup of desktop notifications + for (int id = 1; id <= 128; ++id) { + std::array arguments { + "dbus-send", + "--session", + "--print-reply=literal", + "--dest=org.freedesktop.Notifications", + "/org/freedesktop/Notifications", + "org.freedesktop.Notifications.CloseNotification", + "uint32:" + std::to_string(id), + }; + std::array argv {}; + for (std::size_t i = 0; i < arguments.size(); ++i) { + argv[i] = arguments[i].data(); + } + + posix_spawn_file_actions_t actions; + posix_spawn_file_actions_init(&actions); + posix_spawn_file_actions_addopen(&actions, STDOUT_FILENO, "/dev/null", O_WRONLY, 0); + posix_spawn_file_actions_addopen(&actions, STDERR_FILENO, "/dev/null", O_WRONLY, 0); + + pid_t child = 0; + const int spawn_result = posix_spawnp(&child, arguments[0].c_str(), &actions, nullptr, argv.data(), environ); + posix_spawn_file_actions_destroy(&actions); + if (spawn_result != 0) { + return; + } + waitpid(child, nullptr, 0); + } } } // namespace #endif diff --git a/tests/screenshot_utils.cpp b/tests/screenshot_utils.cpp index f74a2090..5d54730d 100644 --- a/tests/screenshot_utils.cpp +++ b/tests/screenshot_utils.cpp @@ -56,11 +56,7 @@ namespace { static std::once_flag dpiFlag; static bool dpiAware = false; std::call_once(dpiFlag, []() { - using SetProcessDPIAwareFn = BOOL(WINAPI *)(); - auto *fn = reinterpret_cast( // NOSONAR(cpp:S3630): required for GetProcAddress function pointer cast - GetProcAddress(GetModuleHandleA("user32.dll"), "SetProcessDPIAware") - ); - dpiAware = fn == nullptr || fn() == TRUE; + dpiAware = SetProcessDPIAware() == TRUE; }); return dpiAware; } @@ -89,19 +85,24 @@ namespace { namespace screenshot { - inline std::filesystem::path &output_root_ref() { - static std::filesystem::path g_outputRoot; // NOSONAR(cpp:S6018): function-local static is intentional for lazy, TU-local initialization - return g_outputRoot; - } + class ScreenshotState { + public: + static std::filesystem::path &outputRoot() { + return outputRoot_; + } + + private: + inline static std::filesystem::path outputRoot_; + }; void initialize(const std::filesystem::path &rootDir) { - output_root_ref() = rootDir / "screenshots"; + ScreenshotState::outputRoot() = rootDir / "screenshots"; std::error_code ec; - std::filesystem::create_directories(output_root_ref(), ec); + std::filesystem::create_directories(ScreenshotState::outputRoot(), ec); } std::filesystem::path output_root() { - return output_root_ref(); + return ScreenshotState::outputRoot(); } #ifdef __APPLE__ @@ -237,10 +238,10 @@ namespace screenshot { // Add a delay to allow UI elements to render before capturing std::this_thread::sleep_for(std::chrono::milliseconds(500)); - if (output_root_ref().empty()) { + if (ScreenshotState::outputRoot().empty()) { return false; } - auto file = output_root_ref() / (name + ".png"); + auto file = ScreenshotState::outputRoot() / (name + ".png"); #ifdef __APPLE__ return capture_macos(file, options); diff --git a/tests/unit/test_tray.cpp b/tests/unit/test_tray.cpp index 115899b7..0afdf72c 100644 --- a/tests/unit/test_tray.cpp +++ b/tests/unit/test_tray.cpp @@ -5,6 +5,8 @@ #include #include #include +#include +#include #include #include #include @@ -67,41 +69,35 @@ namespace { return {}; } - struct tray_menu g_submenu7_8[] = { // NOSONAR(cpp:S5945,cpp:S5421): C-style array with null sentinel required by tray C API; mutable for runtime callback assignment - {.text = "7", .cb = nullptr}, - {.text = "-"}, - {.text = "8", .cb = nullptr}, - {.text = nullptr} - }; - struct tray_menu g_submenu5_6[] = { // NOSONAR(cpp:S5945,cpp:S5421): C-style array with null sentinel required by tray C API; mutable for runtime callback assignment - {.text = "5", .cb = nullptr}, - {.text = "6", .cb = nullptr}, - {.text = nullptr} - }; - struct tray_menu g_submenu_second[] = { // NOSONAR(cpp:S5945,cpp:S5421): C-style array with null sentinel required by tray C API; mutable for runtime callback assignment - {.text = "THIRD", .submenu = g_submenu7_8}, - {.text = "FOUR", .submenu = g_submenu5_6}, - {.text = nullptr} - }; - struct tray_menu g_submenu[] = { // NOSONAR(cpp:S5945,cpp:S5421): C-style array with null sentinel required by tray C API; mutable for runtime callback assignment - {.text = "Hello", .cb = nullptr}, - {.text = "Checked", .checked = 1, .checkbox = 1, .cb = nullptr}, - {.text = "Disabled", .disabled = 1}, - {.text = "-"}, - {.text = "SubMenu", .submenu = g_submenu_second}, - {.text = "-"}, - {.text = "Quit", .cb = nullptr}, - {.text = nullptr} - }; - struct tray g_testTray = { // NOSONAR(cpp:S5421): mutable global required for shared tray state across TEST_F instances +} // namespace + +class TrayTest: public BaseTest { +private: + static TrayTest &fixtureFor(struct tray_menu *item) { + return *static_cast(item->context); + } + + std::array submenu7_8_ {{{.text = "7", .cb = submenu_cb, .context = this}, {.text = "-"}, {.text = "8", .cb = submenu_cb, .context = this}, {.text = nullptr}}}; + std::array submenu5_6_ {{{.text = "5", .cb = submenu_cb, .context = this}, {.text = "6", .cb = submenu_cb, .context = this}, {.text = nullptr}}}; + std::array submenuSecond_ {{{.text = "THIRD", .context = this, .submenu = submenu7_8_.data()}, {.text = "FOUR", .context = this, .submenu = submenu5_6_.data()}, {.text = nullptr}}}; + std::array submenu_ {{{.text = "Hello", .cb = hello_cb, .context = this}, {.text = "Checked", .checked = 1, .checkbox = 1, .cb = toggle_cb, .context = this}, {.text = "Disabled", .disabled = 1}, {.text = "-"}, {.text = "SubMenu", .context = this, .submenu = submenuSecond_.data()}, {.text = "-"}, {.text = "Quit", .cb = quit_cb, .context = this}, {.text = nullptr}}}; + std::array testTrayStorage_ {}; + struct tray *testTray_ = ::new (static_cast(testTrayStorage_.data())) tray { .icon = TRAY_ICON1, .tooltip = "TestTray", - .menu = g_submenu + .menu = submenu_.data(), + .iconPathCount = 0, }; -} // namespace + bool trayRunning_ {false}; + +protected: + bool &trayRunning = trayRunning_; + struct tray &testTray = *testTray_; + struct tray_menu *const submenu = submenu_.data(); + struct tray_menu *const submenu7_8 = submenu7_8_.data(); + struct tray_menu *const submenu5_6 = submenu5_6_.data(); + struct tray_menu *const submenu_second = submenuSecond_.data(); -class TrayTest: public BaseTest { // NOSONAR(cpp:S3656): fixture members must be protected for TEST_F-generated subclasses -protected: // NOSONAR(cpp:S3656): TEST_F generates subclasses that need access to fixture state/methods void ShutdownTray() { if (!trayRunning) { return; @@ -131,7 +127,7 @@ class TrayTest: public BaseTest { // NOSONAR(cpp:S3656): fixture members must b // Capture a screenshot while the tray menu is open, then dismiss and exit. void captureMenuStateAndExit(const char *screenshotName) const { std::atomic_bool exitRequested {false}; - std::thread capture_thread([this, screenshotName, &exitRequested]() { + std::thread capture_thread([this, screenshotName, &exitRequested]() { // NOSONAR(cpp:S6168): C++17 has no std::jthread and this thread is explicitly joined EXPECT_TRUE(captureScreenshot(screenshotName)); closeMenu(); exitRequested.store(true, std::memory_order_release); @@ -147,43 +143,28 @@ class TrayTest: public BaseTest { // NOSONAR(cpp:S3656): fixture members must b capture_thread.join(); } - bool trayRunning {false}; // NOSONAR(cpp:S3656): protected access required by gtest TEST_F subclass pattern - struct tray &testTray = g_testTray; // NOSONAR(cpp:S3656): protected access required by gtest TEST_F subclass pattern - struct tray_menu *submenu = g_submenu; // NOSONAR(cpp:S3656): protected access required by gtest TEST_F subclass pattern - struct tray_menu *submenu7_8 = g_submenu7_8; // NOSONAR(cpp:S3656): protected access required by gtest TEST_F subclass pattern - struct tray_menu *submenu5_6 = g_submenu5_6; // NOSONAR(cpp:S3656): protected access required by gtest TEST_F subclass pattern - struct tray_menu *submenu_second = g_submenu_second; // NOSONAR(cpp:S3656): protected access required by gtest TEST_F subclass pattern - - static void hello_cb([[maybe_unused]] struct tray_menu *item) { + static void hello_cb(struct tray_menu *) { // Mock implementation } - static void toggle_cb([[maybe_unused]] struct tray_menu *item) { // NOSONAR(cpp:S1172): unused param required by tray_menu.cb function pointer type - g_testTray.menu[1].checked = !g_testTray.menu[1].checked; - tray_update(&g_testTray); + static void toggle_cb(struct tray_menu *item) { + auto &fixture = fixtureFor(item); + item->checked = !item->checked; + tray_update(fixture.testTray_); } - static void quit_cb([[maybe_unused]] struct tray_menu *item) { // NOSONAR(cpp:S1172): unused param required by tray_menu.cb function pointer type + static void quit_cb(struct tray_menu *) { tray_exit(); } - static void submenu_cb([[maybe_unused]] struct tray_menu *item) { // NOSONAR(cpp:S1172): unused param required by tray_menu.cb function pointer type + static void submenu_cb(struct tray_menu *item) { // Mock implementation - tray_update(&g_testTray); + tray_update(fixtureFor(item).testTray_); } void SetUp() override { BaseTest::SetUp(); - // Wire up callbacks (file-scope arrays can't use addresses of class statics at init time) - g_submenu[0].cb = hello_cb; - g_submenu[1].cb = toggle_cb; - g_submenu[6].cb = quit_cb; - g_submenu7_8[0].cb = submenu_cb; - g_submenu7_8[2].cb = submenu_cb; - g_submenu5_6[0].cb = submenu_cb; - g_submenu5_6[1].cb = submenu_cb; - // Skip tests if screenshot tooling is not available if (!ensureScreenshotReady()) { GTEST_SKIP() << "Screenshot tooling missing: " << screenshotUnavailableReason(); @@ -235,8 +216,8 @@ class TrayTest: public BaseTest { // NOSONAR(cpp:S3656): fixture members must b testTray.notification_text = nullptr; testTray.notification_icon = nullptr; testTray.notification_cb = nullptr; - testTray.menu = g_submenu; - g_submenu[1].checked = 1; + testTray.menu = submenu; + submenu[1].checked = 1; } void TearDown() override { @@ -516,7 +497,7 @@ TEST_F(TrayTest, TestCheckboxStates) { EXPECT_EQ(testTray.menu[1].checked, 1); // Show menu open with checkbox in checked state - captureMenuStateAndExit("tray_menu_checkbox_checked"); // NOSONAR(cpp:S6168): helper uses std::thread for AppleClang 17 compatibility + captureMenuStateAndExit("tray_menu_checkbox_checked"); // Re-initialize tray with checkbox unchecked trayRunning = false; @@ -526,7 +507,7 @@ TEST_F(TrayTest, TestCheckboxStates) { ASSERT_EQ(initResult, 0); // Show menu open with checkbox in unchecked state - captureMenuStateAndExit("tray_menu_checkbox_unchecked"); // NOSONAR(cpp:S6168): helper uses std::thread for AppleClang 17 compatibility + captureMenuStateAndExit("tray_menu_checkbox_unchecked"); // Restore initial checked state testTray.menu[1].checked = 1; @@ -600,7 +581,7 @@ TEST_F(TrayTest, TestTrayShowMenu) { ASSERT_EQ(initResult, 0); // Screenshot shows the full menu open, including the SubMenu entry that leads to nested items - captureMenuStateAndExit("tray_menu_shown"); // NOSONAR(cpp:S6168): helper uses std::thread for AppleClang 17 compatibility + captureMenuStateAndExit("tray_menu_shown"); } TEST_F(TrayTest, TestTrayExit) { @@ -617,7 +598,7 @@ TEST_F(TrayTest, TestMenuAppearsOnLeftClick) { trayRunning = (initResult == 0); ASSERT_EQ(initResult, 0); - captureMenuStateAndExit("tray_menu_left_click"); // NOSONAR(cpp:S6168): helper uses std::thread for AppleClang 17 compatibility + captureMenuStateAndExit("tray_menu_left_click"); } TEST_P(TrayNotificationIconTest, TestNotificationCallbackFiredOnClick) { @@ -660,9 +641,8 @@ TEST_F(TrayTest, TestMenuCallbackAfterNotificationUpdate) { static int callbackCount = 0; callbackCount = 0; - auto first_item_callback = [](struct tray_menu *item) { // NOSONAR(cpp:S1172): unused param required by tray_menu.cb function pointer type + auto first_item_callback = [](struct tray_menu *) { callbackCount++; - (void) item; }; void (*original_cb)(struct tray_menu *) = testTray.menu[0].cb; diff --git a/tests/unit/test_tray_qt.cpp b/tests/unit/test_tray_qt.cpp index 4d8eb75f..1b1d9761 100644 --- a/tests/unit/test_tray_qt.cpp +++ b/tests/unit/test_tray_qt.cpp @@ -11,7 +11,7 @@ #include #include #include -#include +#include #include #include #include @@ -50,8 +50,21 @@ namespace { } } // namespace -class TrayQtCoverageTest: public BaseTest { // NOSONAR(cpp:S3656): fixture members/methods are accessed by TEST_F-generated subclasses -protected: // NOSONAR(cpp:S3656): TEST_F requires protected fixture visibility +class TrayQtCoverageTest: public BaseTest { +private: + bool trayRunning_ {false}; + std::array menuItems_ {}; + std::array submenuItems_ {}; + std::vector trayDataStorage_ {}; + struct tray *trayData_ = nullptr; + +protected: + bool &trayRunning = trayRunning_; + std::array &menuItems = menuItems_; + std::array &submenuItems = submenuItems_; + std::vector &trayDataStorage = trayDataStorage_; + struct tray *&trayData = trayData_; + void SetUp() override { BaseTest::SetUp(); @@ -67,17 +80,17 @@ class TrayQtCoverageTest: public BaseTest { // NOSONAR(cpp:S3656): fixture memb menuItems = {{{.text = "Clickable", .cb = menu_item_cb}, {.text = "-"}, {.text = "Submenu", .submenu = submenuItems.data()}, {.text = "Disabled", .disabled = 1, .cb = menu_item_cb}, {.text = "Second Clickable", .cb = menu_item_cb}, {.text = nullptr}}}; trayDataStorage.assign(sizeof(struct tray), std::byte {0}); - trayData = reinterpret_cast(trayDataStorage.data()); // NOSONAR(cpp:S3630): required to map a C flexible-array struct over raw storage - trayData->icon = "icon.png"; - trayData->tooltip = "Qt Tray Coverage"; - trayData->notification_icon = nullptr; - trayData->notification_text = nullptr; - trayData->notification_title = nullptr; - trayData->notification_cb = nullptr; - trayData->menu = menuItems.data(); - - const int iconPathCount = 0; - std::memcpy(const_cast(&trayData->iconPathCount), &iconPathCount, sizeof(iconPathCount)); // NOSONAR(cpp:S859): required to initialize const member in C struct allocated via raw buffer + trayData = ::new (static_cast(trayDataStorage.data())) tray { + .icon = "icon.png", + .tooltip = "Qt Tray Coverage", + .notification_icon = nullptr, + .notification_text = nullptr, + .notification_title = nullptr, + .notification_cb = nullptr, + .cb = nullptr, + .menu = menuItems.data(), + .iconPathCount = 0, + }; } void TearDown() override { @@ -102,12 +115,6 @@ class TrayQtCoverageTest: public BaseTest { // NOSONAR(cpp:S3656): fixture memb tray_loop(0); } } - - bool trayRunning {false}; - std::array menuItems {}; - std::array submenuItems {}; - std::vector trayDataStorage {}; - struct tray *trayData = nullptr; }; #if defined(_WIN32) @@ -285,22 +292,22 @@ TEST_F(TrayQtCoverageTest, ResolveTrayIconFromIconPathArray) { const size_t iconCount = 2; const size_t bufSize = sizeof(struct tray) + iconCount * sizeof(const char *); std::vector buf(bufSize, std::byte {0}); - auto *iconPathTray = reinterpret_cast(buf.data()); // NOSONAR(cpp:S3630): reinterpret_cast is required to map a C flexible-array struct over raw storage - - iconPathTray->icon = "missing-icon-name"; - iconPathTray->tooltip = "Icon path fallback"; - iconPathTray->notification_icon = nullptr; - iconPathTray->notification_text = nullptr; - iconPathTray->notification_title = nullptr; - iconPathTray->notification_cb = nullptr; - iconPathTray->menu = menuItems.data(); - const auto countVal = static_cast(iconCount); - std::memcpy(const_cast(&iconPathTray->iconPathCount), &countVal, sizeof(countVal)); // NOSONAR(cpp:S859): const member initialization is required for this C interop allocation pattern + auto *iconPathTray = ::new (static_cast(buf.data())) tray { + .icon = "missing-icon-name", + .tooltip = "Icon path fallback", + .notification_icon = nullptr, + .notification_text = nullptr, + .notification_title = nullptr, + .notification_cb = nullptr, + .cb = nullptr, + .menu = menuItems.data(), + .iconPathCount = countVal, + }; const char *badIcon = "missing-icon-name"; const char *goodIcon = "icon.png"; - std::memcpy(const_cast(&iconPathTray->allIconPaths[0]), &badIcon, sizeof(badIcon)); // NOSONAR(cpp:S859): required to initialize const flexible-array entries - std::memcpy(const_cast(&iconPathTray->allIconPaths[1]), &goodIcon, sizeof(goodIcon)); // NOSONAR(cpp:S859): required to initialize const flexible-array entries + iconPathTray->allIconPaths[0] = badIcon; + iconPathTray->allIconPaths[1] = goodIcon; const int initResult = tray_init(iconPathTray); trayRunning = (initResult == 0);