From 43746da6f9b3befb9e11191f2cbd04f99e205666 Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Wed, 24 Dec 2025 22:22:47 -0500 Subject: [PATCH 01/19] test: add screenshots --- .github/copilot-instructions.md | 11 + .github/workflows/ci.yml | 66 ++- .github/workflows/publish-screenshots.yml | 58 +++ docs/Doxyfile | 1 + src/tray.h | 17 + src/tray_darwin.m | 28 +- src/tray_linux.c | 54 ++- src/tray_windows.c | 13 +- tests/CMakeLists.txt | 9 + tests/conftest.cpp | 55 ++- tests/screenshot_utils.cpp | 248 +++++++++++ tests/screenshot_utils.h | 19 + tests/unit/test_tray.cpp | 504 +++++++++++++++++++++- 13 files changed, 1059 insertions(+), 24 deletions(-) create mode 100644 .github/copilot-instructions.md create mode 100644 .github/workflows/publish-screenshots.yml create mode 100644 tests/screenshot_utils.cpp create mode 100644 tests/screenshot_utils.h diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 0000000..6b2c3ad --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,11 @@ +On Windows we use msys2 and ucrt64 to compile. +You need to prefix commands with `C:\msys64\msys2_shell.cmd -defterm -here -no-start -ucrt64 -c`. + +Prefix build directories with `cmake-build-`. + +The test executable is named `test_tray` and will be located inside the `tests` directory within +the build directory. + +The project uses gtest as a test framework. + +Always follow the style guidelines defined in .clang-format for c/c++ code. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 62bfbc5..3d5b174 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -49,11 +49,18 @@ jobs: build-essential \ cmake \ ${{ matrix.appindicator }} \ + imagemagick \ libglib2.0-dev \ libnotify-dev \ ninja-build \ xvfb + - name: Setup virtual desktop + if: runner.os == 'Linux' + uses: LizardByte/actions/actions/setup_virtual_desktop@feat/actions/linux-display # todo: pin version + with: + environment: mate + - name: Setup Dependencies macOS if: runner.os == 'macOS' run: | @@ -62,6 +69,7 @@ jobs: cmake \ doxygen \ graphviz \ + imagemagick \ ninja \ node @@ -76,6 +84,7 @@ jobs: mingw-w64-ucrt-x86_64-binutils mingw-w64-ucrt-x86_64-cmake mingw-w64-ucrt-x86_64-graphviz + mingw-w64-ucrt-x86_64-imagemagick mingw-w64-ucrt-x86_64-ninja mingw-w64-ucrt-x86_64-nodejs mingw-w64-ucrt-x86_64-toolchain @@ -98,7 +107,7 @@ jobs: # step output echo "python-path=${python_path}" - echo "python-path=${python_path}" >> $GITHUB_OUTPUT + echo "python-path=${python_path}" >> "${GITHUB_OUTPUT}" - name: Build run: | @@ -119,18 +128,57 @@ jobs: -S . ninja -C build + - name: Init tray icon (Windows) + if: runner.os == 'Windows' + working-directory: build/tests + run: ./test_tray --gtest_color=yes --gtest_filter=TrayTest.TestTrayInit + + - name: Configure Windows + if: runner.os == 'Windows' + shell: pwsh + run: | + echo "::group::Enable all tray icons" + Invoke-WebRequest ` + -Uri "https://raw.githubusercontent.com/paulmann/windows-show-all-tray-icons/main/Enable-AllTrayIcons.ps1" ` + -OutFile "Enable-AllTrayIcons.ps1" + .\Enable-AllTrayIcons.ps1 -Action Enable -Force # Enable with comprehensive method (resets ALL icon settings) + echo "::endgroup::" + + echo "::group::Disable Do Not Disturb" + Add-Type -AssemblyName System.Windows.Forms + Start-Process "ms-settings:notifications" + Start-Sleep -Seconds 2 + [System.Windows.Forms.SendKeys]::SendWait("{TAB}") + [System.Windows.Forms.SendKeys]::SendWait("{TAB}") + [System.Windows.Forms.SendKeys]::SendWait(" ") + echo "::endgroup::" + + echo "::group::Minimize all windows" + $shell = New-Object -ComObject Shell.Application + $shell.MinimizeAll() + echo "::endgroup::" + + echo "::group::Set Date - Hack for Quiet Time" + $newDate = (Get-Date).AddHours(2) + Set-Date -Date $newDate + echo "::endgroup::" + - name: Run tests id: test # TODO: tests randomly hang on Linux, https://github.com/LizardByte/tray/issues/45 - timeout-minutes: 1 + timeout-minutes: 3 working-directory: build/tests - run: | - if [ "${{ runner.os }}" = "Linux" ]; then - export DISPLAY=:1 - Xvfb ${DISPLAY} -screen 0 1024x768x24 & - fi + run: ./test_tray --gtest_color=yes --gtest_output=xml:test_results.xml - ./test_tray --gtest_color=yes --gtest_output=xml:test_results.xml + - name: Upload screenshots + if: >- + always() && + (steps.test.outcome == 'success' || steps.test.outcome == 'failure') + uses: actions/upload-artifact@v6 + with: + name: tray-screenshots-${{ runner.os }}${{ matrix.appindicator && format('-{0}', matrix.appindicator) || '' }} + path: build/tests/screenshots + if-no-files-found: error - name: Generate gcov report id: test_report @@ -159,7 +207,7 @@ jobs: if [ -n "${{ matrix.appindicator }}" ]; then flags="${flags},${{ matrix.appindicator }}" fi - echo "flags=${flags}" >> $GITHUB_OUTPUT + echo "flags=${flags}" >> "${GITHUB_OUTPUT}" - name: Upload coverage # any except canceled or skipped diff --git a/.github/workflows/publish-screenshots.yml b/.github/workflows/publish-screenshots.yml new file mode 100644 index 0000000..efa0678 --- /dev/null +++ b/.github/workflows/publish-screenshots.yml @@ -0,0 +1,58 @@ +--- +name: Publish Screenshots + +on: + workflow_run: + workflows: ["CI"] + types: + - completed + +permissions: + contents: write + pull-requests: write + +jobs: + publish: + if: github.event.workflow_run.conclusion == 'success' + runs-on: ubuntu-latest + steps: + - name: Download Artifacts + uses: actions/download-artifact@v7 + with: + path: screenshots + pattern: tray-screenshots-* + run-id: ${{ github.event.workflow_run.id }} + + - name: Debug screenshots + run: ls -R screenshots + + - name: Determine Branch and Path + id: determine + env: + PULL_REQUEST_NUMBER: ${{ github.event.workflow_run.pull_requests[0].number }} + HEAD_BRANCH: ${{ github.event.workflow_run.head_branch }} + run: | + if [ -n "${PULL_REQUEST_NUMBER}" ]; then + PR_NUMBER=${PULL_REQUEST_NUMBER} + BRANCH_PATH="PR-${PULL_REQUEST_NUMBER}" + is_pr=true + else + BRANCH_NAME=$(echo "${HEAD_BRANCH}" | sed 's/\//-/g') + BRANCH_PATH="${BRANCH_NAME}" + is_pr=false + fi + + { + echo "branch_path=${BRANCH_PATH}" + echo "is_pr=${is_pr}" + echo "pr_number=${PR_NUMBER}" + } >> "${GITHUB_OUTPUT}" + + # debug outputs + cat "${GITHUB_OUTPUT}" + + - name: Checkout Screenshots Branch + uses: actions/checkout@v6 + with: + ref: screenshots + path: screenshots-repo diff --git a/docs/Doxyfile b/docs/Doxyfile index 77aaefb..3f004c6 100644 --- a/docs/Doxyfile +++ b/docs/Doxyfile @@ -31,6 +31,7 @@ PROJECT_NAME = tray DOT_GRAPH_MAX_NODES = 50 IMAGE_PATH = ../docs/images INCLUDE_PATH = +PREDEFINED += TRAY_WINAPI # files and directories to process USE_MDFILE_AS_MAINPAGE = ../README.md diff --git a/src/tray.h b/src/tray.h index ce28ef1..164a436 100644 --- a/src/tray.h +++ b/src/tray.h @@ -5,6 +5,10 @@ #ifndef TRAY_H #define TRAY_H +#if defined(TRAY_WINAPI) + #include +#endif + #ifdef __cplusplus extern "C" { #endif @@ -64,11 +68,24 @@ extern "C" { */ void tray_update(struct tray *tray); + /** + * @brief Force show the tray menu (for testing purposes). + */ + void tray_show_menu(void); + /** * @brief Terminate UI loop. */ void tray_exit(void); +#if defined(TRAY_WINAPI) + /** + * @brief Get the tray window handle. + * @return The window handle. + */ + HWND tray_get_hwnd(void); +#endif + #ifdef __cplusplus } // extern "C" #endif diff --git a/src/tray_darwin.m b/src/tray_darwin.m index 4c644d2..b376c2d 100644 --- a/src/tray_darwin.m +++ b/src/tray_darwin.m @@ -39,9 +39,26 @@ - (IBAction)menuCallback:(id)sender { static NSApplication *app; static NSStatusBar *statusBar; static NSStatusItem *statusItem; +static int loopResult = 0; #define QUIT_EVENT_SUBTYPE 0x0DED ///< NSEvent subtype used to signal exit. +static void drain_quit_events(void) { + while (YES) { + NSEvent *event = [app nextEventMatchingMask:ULONG_MAX + untilDate:[NSDate distantPast] + inMode:[NSString stringWithUTF8String:"kCFRunLoopDefaultMode"] + dequeue:TRUE]; + if (event == nil) { + break; + } + if (event.type == NSEventTypeApplicationDefined && event.subtype == QUIT_EVENT_SUBTYPE) { + continue; + } + [app sendEvent:event]; + } +} + static NSMenu *_tray_menu(struct tray_menu *m) { NSMenu *menu = [[NSMenu alloc] init]; [menu setAutoenablesItems:FALSE]; @@ -67,6 +84,7 @@ - (IBAction)menuCallback:(id)sender { } int tray_init(struct tray *tray) { + loopResult = 0; AppDelegate *delegate = [[AppDelegate alloc] init]; app = [NSApplication sharedApplication]; [app setDelegate:delegate]; @@ -74,6 +92,7 @@ int tray_init(struct tray *tray) { statusItem = [statusBar statusItemWithLength:NSVariableStatusItemLength]; tray_update(tray); [app activateIgnoringOtherApps:TRUE]; + drain_quit_events(); return 0; } @@ -85,12 +104,13 @@ int tray_loop(int blocking) { dequeue:TRUE]; if (event) { if (event.type == NSEventTypeApplicationDefined && event.subtype == QUIT_EVENT_SUBTYPE) { - return -1; + loopResult = -1; + return loopResult; } [app sendEvent:event]; } - return 0; + return loopResult; } void tray_update(struct tray *tray) { @@ -101,6 +121,10 @@ void tray_update(struct tray *tray) { [statusItem setMenu:_tray_menu(tray->menu)]; } +void tray_show_menu(void) { + [statusItem popUpStatusItemMenu:statusItem.menu]; +} + void tray_exit(void) { NSEvent *event = [NSEvent otherEventWithType:NSEventTypeApplicationDefined location:NSMakePoint(0, 0) diff --git a/src/tray_linux.c b/src/tray_linux.c index 4678c5e..3efc7eb 100644 --- a/src/tray_linux.c +++ b/src/tray_linux.c @@ -5,7 +5,9 @@ // standard includes #include #include +#include #include +#include // lib includes #ifdef TRAY_AYATANA_APPINDICATOR @@ -17,7 +19,10 @@ #define IS_APP_INDICATOR APP_IS_INDICATOR ///< Define IS_APP_INDICATOR for app-indicator compatibility. #endif #include -#define TRAY_APPINDICATOR_ID "tray-id" ///< Tray appindicator ID. + +// Use a per-process AppIndicator id to avoid DBus collisions when tests create multiple +// tray instances in the same desktop/session. +static unsigned long tray_appindicator_seq = 0; // local includes #include "tray.h" @@ -29,6 +34,7 @@ static pthread_mutex_t async_update_mutex = PTHREAD_MUTEX_INITIALIZER; static AppIndicator *indicator = NULL; static int loop_result = 0; static NotifyNotification *currentNotification = NULL; +static GtkMenu *current_menu = NULL; static void _tray_menu_cb(GtkMenuItem *item, gpointer data) { (void) item; @@ -67,8 +73,23 @@ int tray_init(struct tray *tray) { if (gtk_init_check(0, NULL) == FALSE) { return -1; } + + // If a previous tray instance wasn't fully torn down (common in unit tests), + // drop our references before creating a new indicator. + if (indicator != NULL) { + g_object_unref(G_OBJECT(indicator)); + indicator = NULL; + } + loop_result = 0; notify_init("tray-icon"); - indicator = app_indicator_new(TRAY_APPINDICATOR_ID, tray->icon, APP_INDICATOR_CATEGORY_APPLICATION_STATUS); + // The id is used as part of the exported DBus object path. + // Make it unique per *tray instance* to prevent collisions inside a single test process. + // Avoid underscores and other characters that may be normalized/stripped. + char appindicator_id[64]; + tray_appindicator_seq++; + snprintf(appindicator_id, sizeof(appindicator_id), "trayid%ld%lu", (long) getpid(), tray_appindicator_seq); + + indicator = app_indicator_new(appindicator_id, tray->icon, APP_INDICATOR_CATEGORY_APPLICATION_STATUS); if (indicator == NULL || !IS_APP_INDICATOR(indicator)) { return -1; } @@ -89,7 +110,13 @@ static gboolean tray_update_internal(gpointer user_data) { app_indicator_set_icon_full(indicator, tray->icon, tray->icon); // GTK is all about reference counting, so previous menu should be destroyed // here - app_indicator_set_menu(indicator, GTK_MENU(_tray_menu(tray->menu))); + GtkMenu *menu = GTK_MENU(_tray_menu(tray->menu)); + app_indicator_set_menu(indicator, menu); + if (current_menu != NULL) { + g_object_unref(current_menu); + } + current_menu = menu; + g_object_ref(current_menu); // Keep a reference for showing } if (tray->notification_text != 0 && strlen(tray->notification_text) > 0 && notify_is_initted()) { if (currentNotification != NULL && NOTIFY_IS_NOTIFICATION(currentNotification)) { @@ -144,12 +171,33 @@ void tray_update(struct tray *tray) { } } +void tray_show_menu(void) { + if (current_menu != NULL) { + gtk_menu_popup(current_menu, NULL, NULL, NULL, NULL, 0, gtk_get_current_event_time()); + } +} + static gboolean tray_exit_internal(gpointer user_data) { + (void) user_data; + if (currentNotification != NULL && NOTIFY_IS_NOTIFICATION(currentNotification)) { int v = notify_notification_close(currentNotification, NULL); if (v == TRUE) { g_object_unref(G_OBJECT(currentNotification)); } + currentNotification = NULL; + } + + if (current_menu != NULL) { + g_object_unref(current_menu); + current_menu = NULL; + } + + if (indicator != NULL) { + // Make the indicator passive before unref to encourage a clean DBus unexport. + app_indicator_set_status(indicator, APP_INDICATOR_STATUS_PASSIVE); + g_object_unref(G_OBJECT(indicator)); + indicator = NULL; } notify_uninit(); return G_SOURCE_REMOVE; diff --git a/src/tray_windows.c b/src/tray_windows.c index 60de5a8..1f66588 100644 --- a/src/tray_windows.c +++ b/src/tray_windows.c @@ -3,9 +3,9 @@ * @brief System tray implementation for Windows. */ // standard includes -#include +#include // clang-format off -// build fails if shellapi.h is included before windows.h +// build fails if shellapi.h is included before Windows.h #include // clang-format on @@ -315,6 +315,11 @@ void tray_update(struct tray *tray) { } } +void tray_show_menu(void) { + // Simulate a right-click on the tray icon to show the menu + SendMessage(hwnd, WM_TRAY_CALLBACK_MESSAGE, 0, WM_RBUTTONUP); +} + void tray_exit(void) { Shell_NotifyIconW(NIM_DELETE, &nid); SendMessage(hwnd, WM_CLOSE, 0, 0); @@ -324,3 +329,7 @@ void tray_exit(void) { } UnregisterClass(WC_TRAY_CLASS_NAME, GetModuleHandle(NULL)); } + +HWND tray_get_hwnd(void) { + return hwnd; +} diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 4474708..f12b98a 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -18,9 +18,17 @@ if (WIN32) set(gtest_force_shared_crt ON CACHE BOOL "" FORCE) # cmake-lint: disable=C0103 endif () +# extra libraries for tests +if (APPLE) + set(TEST_LIBS "-framework Cocoa") +elseif (WIN32) + set(TEST_LIBS gdi32 gdiplus) +endif() + file(GLOB_RECURSE TEST_SOURCES ${CMAKE_SOURCE_DIR}/tests/conftest.cpp ${CMAKE_SOURCE_DIR}/tests/utils.cpp + ${CMAKE_SOURCE_DIR}/tests/screenshot_utils.cpp ${CMAKE_SOURCE_DIR}/tests/test_*.cpp) add_executable(${PROJECT_NAME} @@ -29,6 +37,7 @@ add_executable(${PROJECT_NAME} set_target_properties(${PROJECT_NAME} PROPERTIES CXX_STANDARD 17) target_link_directories(${PROJECT_NAME} PRIVATE ${TRAY_EXTERNAL_DIRECTORIES}) target_link_libraries(${PROJECT_NAME} + ${TEST_LIBS} ${TRAY_EXTERNAL_LIBRARIES} gtest gtest_main) # if we use this we don't need our own main function diff --git a/tests/conftest.cpp b/tests/conftest.cpp index 6f51eac..64ad934 100644 --- a/tests/conftest.cpp +++ b/tests/conftest.cpp @@ -1,11 +1,13 @@ // standard includes #include #include +#include // lib includes #include // test includes +#include "tests/screenshot_utils.h" #include "tests/utils.h" // Undefine the original TEST macro @@ -34,7 +36,8 @@ class BaseTest: public ::testing::Test { BaseTest(): sbuf {nullptr}, pipe_stdout {nullptr}, - pipe_stderr {nullptr} { + pipe_stderr {nullptr}, + screenshotsReady {false} { // intentionally empty } @@ -59,6 +62,8 @@ class BaseTest: public ::testing::Test { testBinaryDir = std::filesystem::current_path(); } + initializeScreenshotsOnce(); + sbuf = std::cout.rdbuf(); // save cout buffer (std::cout) std::cout.rdbuf(cout_buffer.rdbuf()); // redirect cout to buffer (std::cout) } @@ -102,6 +107,19 @@ class BaseTest: public ::testing::Test { std::streambuf *sbuf; FILE *pipe_stdout; FILE *pipe_stderr; + bool screenshotsReady; + + void initializeScreenshotsOnce() { + static std::once_flag screenshotInitFlag; + std::call_once(screenshotInitFlag, [this]() { + auto root = testBinaryDir; + if (!root.empty()) { + std::error_code ec; + std::filesystem::remove_all(root / "screenshots", ec); + } + screenshot::initialize(root); + }); + } int exec(const char *cmd) { std::array buffer {}; @@ -124,6 +142,41 @@ class BaseTest: public ::testing::Test { } return returnCode; } + + bool ensureScreenshotReady() { + if (screenshotsReady) { + return true; + } + std::string reason; + if (!screenshot::is_available(&reason)) { + screenshotUnavailableReason = reason; + return false; + } + auto root = screenshot::output_root(); + if (root.empty()) { + screenshotUnavailableReason = "Screenshot output directory not initialized"; + return false; + } + screenshotsReady = true; + return true; + } + + bool captureScreenshot(const std::string &name) { + if (!screenshotsReady) { + return false; + } + bool ok = screenshot::capture(name); + if (!ok) { + std::cout << "Failed to capture screenshot: " << name << std::endl; + } + return ok; + } + + std::filesystem::path screenshotsRoot() const { + return screenshot::output_root(); + } + + std::string screenshotUnavailableReason; }; class LinuxTest: public BaseTest { diff --git a/tests/screenshot_utils.cpp b/tests/screenshot_utils.cpp new file mode 100644 index 0000000..cbed19c --- /dev/null +++ b/tests/screenshot_utils.cpp @@ -0,0 +1,248 @@ +// test includes +#include "screenshot_utils.h" + +// standard includes +#include +#include +#include +#include +#include +#include +#include +#include +#include +#ifdef _WIN32 + #ifndef NOMINMAX + #define NOMINMAX + #endif + #include +// clang-format off + // build fails if PropIdl.h and gdiplus.h are included before Windows.h + #include + #include +// clang-format on +#endif + +namespace { + std::filesystem::path g_outputRoot; + + std::string quote_shell_path(const std::filesystem::path &path) { + std::string input = path.string(); + std::string output; + output.reserve(input.size() + 2); + output.push_back('"'); + for (char ch : input) { + if (ch == '"') { + output.append("\\\""); + } else { + output.push_back(ch); + } + } + output.push_back('"'); + return output; + } + +#ifdef _WIN32 + std::once_flag gdiplusInitFlag; + bool gdiplusReady = false; + ULONG_PTR gdiplusToken = 0; + std::once_flag dpiFlag; + bool dpiAware = false; + + bool ensure_gdiplus() { + std::call_once(gdiplusInitFlag, []() { + Gdiplus::GdiplusStartupInput input; + gdiplusReady = Gdiplus::GdiplusStartup(&gdiplusToken, &input, nullptr) == Gdiplus::Ok; + }); + return gdiplusReady; + } + + bool ensure_dpi_awareness() { + std::call_once(dpiFlag, []() { + auto setDPIAware = reinterpret_cast(GetProcAddress(GetModuleHandleA("user32.dll"), "SetProcessDPIAware")); + dpiAware = setDPIAware == nullptr || setDPIAware() == TRUE; + }); + return dpiAware; + } + + bool png_encoder_clsid(CLSID *clsid) { + UINT num = 0; + UINT size = 0; + if (Gdiplus::GetImageEncodersSize(&num, &size) != Gdiplus::Ok || size == 0) { + return false; + } + std::vector buffer(size); + auto info = reinterpret_cast(buffer.data()); + if (Gdiplus::GetImageEncoders(num, size, info) != Gdiplus::Ok) { + return false; + } + for (UINT i = 0; i < num; ++i) { + if (wcscmp(info[i].MimeType, L"image/png") == 0) { + *clsid = info[i].Clsid; + return true; + } + } + return false; + } + +#endif +} // namespace + +namespace screenshot { + + void initialize(const std::filesystem::path &rootDir) { + g_outputRoot = rootDir / "screenshots"; + std::error_code ec; + std::filesystem::create_directories(g_outputRoot, ec); + } + + std::filesystem::path output_root() { + return g_outputRoot; + } + +#ifdef __APPLE__ + static bool capture_macos(const std::filesystem::path &file, const Options &) { + std::string cmd = "screencapture -x " + quote_shell_path(file); + return std::system(cmd.c_str()) == 0; + } +#endif + +#ifdef __linux__ + static bool capture_linux(const std::filesystem::path &file, const Options &) { + std::string target = quote_shell_path(file); + if (std::system("which import > /dev/null 2>&1") == 0) { + std::string cmd = "import -window root " + target; + if (std::system(cmd.c_str()) == 0) { + return true; + } + } + std::string cmd = "gnome-screenshot -f " + target; + return std::system(cmd.c_str()) == 0; + } +#endif + +#ifdef _WIN32 + static bool capture_windows(const std::filesystem::path &file, const Options &) { + if (!ensure_dpi_awareness()) { + std::cerr << "Failed to enable DPI awareness" << std::endl; + return false; + } + if (!ensure_gdiplus()) { + std::cerr << "GDI+ initialization failed" << std::endl; + return false; + } + + int left = GetSystemMetrics(SM_XVIRTUALSCREEN); + int top = GetSystemMetrics(SM_YVIRTUALSCREEN); + int width = GetSystemMetrics(SM_CXVIRTUALSCREEN); + int height = GetSystemMetrics(SM_CYVIRTUALSCREEN); + + HWND desktop = GetDesktopWindow(); + if ((width <= 0 || height <= 0) && desktop != nullptr) { + RECT rect {}; + if (GetWindowRect(desktop, &rect)) { + left = rect.left; + top = rect.top; + width = rect.right - rect.left; + height = rect.bottom - rect.top; + } + } + if (width <= 0 || height <= 0) { + std::cerr << "Desktop dimensions invalid" << std::endl; + return false; + } + + HDC hdcScreen = GetDC(nullptr); + if (hdcScreen == nullptr) { + std::cerr << "GetDC(nullptr) failed" << std::endl; + return false; + } + HDC hdcMem = CreateCompatibleDC(hdcScreen); + if (hdcMem == nullptr) { + std::cerr << "CreateCompatibleDC failed" << std::endl; + ReleaseDC(nullptr, hdcScreen); + return false; + } + HBITMAP hbm = CreateCompatibleBitmap(hdcScreen, width, height); + if (hbm == nullptr) { + std::cerr << "CreateCompatibleBitmap failed" << std::endl; + DeleteDC(hdcMem); + ReleaseDC(nullptr, hdcScreen); + return false; + } + HGDIOBJ old = SelectObject(hdcMem, hbm); + BOOL ok = BitBlt(hdcMem, 0, 0, width, height, hdcScreen, left, top, SRCCOPY | CAPTUREBLT); + SelectObject(hdcMem, old); + DeleteDC(hdcMem); + ReleaseDC(nullptr, hdcScreen); + if (!ok) { + std::cerr << "BitBlt failed with error " << GetLastError() << std::endl; + DeleteObject(hbm); + return false; + } + + Gdiplus::Bitmap bitmap(hbm, nullptr); + DeleteObject(hbm); + + CLSID pngClsid; + if (!png_encoder_clsid(&pngClsid)) { + std::cerr << "PNG encoder CLSID not found" << std::endl; + return false; + } + std::wstring widePath = file.wstring(); + if (bitmap.Save(widePath.c_str(), &pngClsid, nullptr) != Gdiplus::Ok) { + std::cerr << "GDI+ failed to write " << file << std::endl; + return false; + } + return true; + } +#endif + + bool is_available(std::string *reason) { +#ifdef __APPLE__ + return true; +#elif defined(__linux__) + if (std::system("which import > /dev/null 2>&1") == 0 || std::system("which gnome-screenshot > /dev/null 2>&1") == 0) { + return true; + } + if (reason) { + *reason = "Neither ImageMagick 'import' nor gnome-screenshot found"; + } + return false; +#elif defined(_WIN32) + if (ensure_gdiplus()) { + return true; + } + if (reason) { + *reason = "Failed to initialize GDI+"; + } + return false; +#else + if (reason) { + *reason = "Unsupported platform"; + } + return false; +#endif + } + + bool capture(const std::string &name, const Options &options) { + // Add a delay to allow UI elements to render before capturing + std::this_thread::sleep_for(std::chrono::milliseconds(500)); + + if (g_outputRoot.empty()) { + return false; + } + auto file = g_outputRoot / (name + ".png"); + +#ifdef __APPLE__ + return capture_macos(file, options); +#elif defined(__linux__) + return capture_linux(file, options); +#elif defined(_WIN32) + return capture_windows(file, options); +#else + return false; +#endif + } + +} // namespace screenshot diff --git a/tests/screenshot_utils.h b/tests/screenshot_utils.h new file mode 100644 index 0000000..df98641 --- /dev/null +++ b/tests/screenshot_utils.h @@ -0,0 +1,19 @@ +#pragma once + +// standard includes +#include +#include +#include + +namespace screenshot { + + struct Options { + std::optional region; // reserved for future ROI support + }; + + void initialize(const std::filesystem::path &rootDir); + bool is_available(std::string *reason = nullptr); + bool capture(const std::string &name, const Options &options = {}); + std::filesystem::path output_root(); + +} // namespace screenshot diff --git a/tests/unit/test_tray.cpp b/tests/unit/test_tray.cpp index ab9560f..509db28 100644 --- a/tests/unit/test_tray.cpp +++ b/tests/unit/test_tray.cpp @@ -1,16 +1,27 @@ // test includes #include "tests/conftest.cpp" +// standard includes +#include +#include + #if defined(_WIN32) || defined(_WIN64) + #include +// clang-format off + // build fails if shellapi.h is included before Windows.h + #include + // clang-format on #define TRAY_WINAPI 1 #elif defined(__linux__) || defined(linux) || defined(__linux) #define TRAY_APPINDICATOR 1 #elif defined(__APPLE__) || defined(__MACH__) + #include #define TRAY_APPKIT 1 #endif // local includes #include "src/tray.h" +#include "tests/screenshot_utils.h" #if TRAY_APPINDICATOR #define TRAY_ICON1 "mail-message-new" @@ -26,6 +37,9 @@ class TrayTest: public BaseTest { protected: static struct tray testTray; + bool trayRunning; + std::string iconPath1; + std::string iconPath2; // Static arrays for submenus static struct tray_menu submenu7_8[]; @@ -53,13 +67,81 @@ class TrayTest: public BaseTest { } void SetUp() override { + BaseTest::SetUp(); + + // Skip tests if screenshot tooling is not available + if (!ensureScreenshotReady()) { + GTEST_SKIP() << "Screenshot tooling missing: " << screenshotUnavailableReason; + } + if (screenshot::output_root().empty()) { + GTEST_SKIP() << "Screenshot output path not initialized"; + } + +#if defined(TRAY_WINAPI) || defined(TRAY_APPKIT) + // Ensure icon files exist in test binary directory + // Look for icons in project root or cmake build directory + std::filesystem::path projectRoot = testBinaryDir.parent_path(); + std::filesystem::path iconSource; + + // Try icons directory first + if (std::filesystem::exists(projectRoot / "icons" / TRAY_ICON1)) { + iconSource = projectRoot / "icons" / TRAY_ICON1; + } + // Try project root + else if (std::filesystem::exists(projectRoot / TRAY_ICON1)) { + iconSource = projectRoot / TRAY_ICON1; + } + // Try current directory + else if (std::filesystem::exists(std::filesystem::path(TRAY_ICON1))) { + iconSource = std::filesystem::path(TRAY_ICON1); + } + + // Copy icon to test binary directory if not already there + if (!iconSource.empty()) { + std::filesystem::path iconDest = testBinaryDir / TRAY_ICON1; + if (!std::filesystem::exists(iconDest)) { + std::error_code ec; + std::filesystem::copy_file(iconSource, iconDest, ec); + if (ec) { + std::cout << "Warning: Failed to copy icon file: " << ec.message() << std::endl; + } + } + } +#endif + + trayRunning = false; testTray.icon = TRAY_ICON1; testTray.tooltip = "TestTray"; testTray.menu = submenu; + submenu[1].checked = 1; // Reset checkbox state to initial value } void TearDown() override { - // Clean up any resources if needed + ShutdownTray(); + BaseTest::TearDown(); + } + + void ShutdownTray() { + if (!trayRunning) { + return; + } + tray_exit(); + tray_loop(0); + trayRunning = false; + } + + // Process pending GTK events to allow AppIndicator to register + // Call this ONLY before screenshots to ensure the icon is visible + void WaitForTrayReady() { +#if defined(TRAY_APPINDICATOR) + // On Linux with AppIndicator, process GTK events to allow D-Bus registration + // This ensures the icon actually appears in the system tray before screenshots + // Use shorter iterations to avoid interfering with event loop state + for (int i = 0; i < 100; i++) { + tray_loop(0); // Non-blocking - process pending events + std::this_thread::sleep_for(std::chrono::milliseconds(5)); + } +#endif } }; @@ -98,25 +180,35 @@ struct tray TrayTest::testTray = { TEST_F(TrayTest, TestTrayInit) { int result = tray_init(&testTray); - EXPECT_EQ(result, 0); // make sure return value is 0 + trayRunning = (result == 0); + EXPECT_EQ(result, 0); + WaitForTrayReady(); + EXPECT_TRUE(captureScreenshot("tray_icon_initial")); } TEST_F(TrayTest, TestTrayLoop) { + int initResult = tray_init(&testTray); + trayRunning = (initResult == 0); + ASSERT_EQ(initResult, 0); int result = tray_loop(1); - EXPECT_EQ(result, 0); // make sure return value is 0 + EXPECT_EQ(result, 0); + WaitForTrayReady(); + EXPECT_TRUE(captureScreenshot("tray_loop_iteration")); } TEST_F(TrayTest, TestTrayUpdate) { - // check the initial values + int initResult = tray_init(&testTray); + trayRunning = (initResult == 0); + ASSERT_EQ(initResult, 0); EXPECT_EQ(testTray.icon, TRAY_ICON1); - EXPECT_EQ(testTray.tooltip, "TestTray"); // update the values testTray.icon = TRAY_ICON2; testTray.tooltip = "TestTray2"; tray_update(&testTray); EXPECT_EQ(testTray.icon, TRAY_ICON2); - EXPECT_EQ(testTray.tooltip, "TestTray2"); + WaitForTrayReady(); + EXPECT_TRUE(captureScreenshot("tray_icon_updated")); // put back the original values testTray.icon = TRAY_ICON1; @@ -127,12 +219,410 @@ TEST_F(TrayTest, TestTrayUpdate) { } TEST_F(TrayTest, TestToggleCallback) { + int initResult = tray_init(&testTray); + trayRunning = (initResult == 0); + ASSERT_EQ(initResult, 0); bool initialCheckedState = testTray.menu[1].checked; toggle_cb(&testTray.menu[1]); EXPECT_EQ(testTray.menu[1].checked, !initialCheckedState); + WaitForTrayReady(); + EXPECT_TRUE(captureScreenshot("tray_menu_toggle")); +} + +TEST_F(TrayTest, TestMenuItemCallback) { + int initResult = tray_init(&testTray); + trayRunning = (initResult == 0); + ASSERT_EQ(initResult, 0); + + // Test hello callback - it should work without crashing + ASSERT_NE(testTray.menu[0].cb, nullptr); + testTray.menu[0].cb(&testTray.menu[0]); + tray_loop(1); + WaitForTrayReady(); + EXPECT_TRUE(captureScreenshot("tray_menu_callback_hello")); +} + +TEST_F(TrayTest, TestDisabledMenuItem) { + int initResult = tray_init(&testTray); + trayRunning = (initResult == 0); + ASSERT_EQ(initResult, 0); + + // Verify disabled menu item + EXPECT_EQ(testTray.menu[2].disabled, 1); + EXPECT_STREQ(testTray.menu[2].text, "Disabled"); + tray_loop(1); + WaitForTrayReady(); + EXPECT_TRUE(captureScreenshot("tray_menu_disabled_item")); +} + +TEST_F(TrayTest, TestMenuSeparator) { + int initResult = tray_init(&testTray); + trayRunning = (initResult == 0); + ASSERT_EQ(initResult, 0); + + // Verify separator exists + EXPECT_STREQ(testTray.menu[3].text, "-"); + EXPECT_EQ(testTray.menu[3].cb, nullptr); + tray_loop(1); + WaitForTrayReady(); + EXPECT_TRUE(captureScreenshot("tray_menu_with_separator")); +} + +TEST_F(TrayTest, TestSubmenuStructure) { + int initResult = tray_init(&testTray); + trayRunning = (initResult == 0); + ASSERT_EQ(initResult, 0); + + // Verify submenu structure + EXPECT_STREQ(testTray.menu[4].text, "SubMenu"); + ASSERT_NE(testTray.menu[4].submenu, nullptr); + + // Verify nested submenu levels + EXPECT_STREQ(testTray.menu[4].submenu[0].text, "THIRD"); + ASSERT_NE(testTray.menu[4].submenu[0].submenu, nullptr); + EXPECT_STREQ(testTray.menu[4].submenu[0].submenu[0].text, "7"); + + tray_loop(1); + WaitForTrayReady(); + EXPECT_TRUE(captureScreenshot("tray_submenu_structure")); +} + +TEST_F(TrayTest, TestSubmenuCallback) { + int initResult = tray_init(&testTray); + trayRunning = (initResult == 0); + ASSERT_EQ(initResult, 0); + + // Test submenu callback + ASSERT_NE(testTray.menu[4].submenu[0].submenu[0].cb, nullptr); + testTray.menu[4].submenu[0].submenu[0].cb(&testTray.menu[4].submenu[0].submenu[0]); + tray_loop(1); + WaitForTrayReady(); + EXPECT_TRUE(captureScreenshot("tray_submenu_callback_executed")); +} + +TEST_F(TrayTest, TestNotificationDisplay) { +#if !(defined(_WIN32) || defined(__linux__) || defined(__APPLE__)) + GTEST_SKIP() << "Notifications only supported on desktop platforms"; +#endif + +#if defined(_WIN32) + QUERY_USER_NOTIFICATION_STATE notification_state; + HRESULT ns = SHQueryUserNotificationState(¬ification_state); + if (ns != S_OK || notification_state != QUNS_ACCEPTS_NOTIFICATIONS) { + GTEST_SKIP() << "Notifications not accepted in this environment. SHQueryUserNotificationState result: " << ns << ", state: " << notification_state; + } +#endif + + int initResult = tray_init(&testTray); + trayRunning = (initResult == 0); + ASSERT_EQ(initResult, 0); + + // Set notification properties + testTray.notification_title = "Test Notification"; + testTray.notification_text = "This is a test notification message"; + testTray.notification_icon = TRAY_ICON1; + + tray_update(&testTray); + tray_loop(1); + + WaitForTrayReady(); + EXPECT_TRUE(captureScreenshot("tray_notification_displayed")); + + // Clear notification + testTray.notification_title = nullptr; + testTray.notification_text = nullptr; + testTray.notification_icon = nullptr; + tray_update(&testTray); +} + +TEST_F(TrayTest, TestNotificationCallback) { +#if !(defined(_WIN32) || defined(__linux__) || defined(__APPLE__)) + GTEST_SKIP() << "Notifications only supported on desktop platforms"; +#endif + +#if defined(_WIN32) + QUERY_USER_NOTIFICATION_STATE notification_state; + HRESULT ns = SHQueryUserNotificationState(¬ification_state); + if (ns != S_OK || notification_state != QUNS_ACCEPTS_NOTIFICATIONS) { + GTEST_SKIP() << "Notifications not accepted in this environment. SHQueryUserNotificationState result: " << ns << ", state: " << notification_state; + } +#endif + + static bool callbackInvoked = false; + auto notification_callback = []() { + callbackInvoked = true; + }; + + int initResult = tray_init(&testTray); + trayRunning = (initResult == 0); + ASSERT_EQ(initResult, 0); + + // Set notification with callback + testTray.notification_title = "Clickable Notification"; + testTray.notification_text = "Click this notification to test callback"; + testTray.notification_icon = TRAY_ICON1; + testTray.notification_cb = notification_callback; + + tray_update(&testTray); + tray_loop(1); + + WaitForTrayReady(); + EXPECT_TRUE(captureScreenshot("tray_notification_with_callback")); + + // Note: callback would be invoked by user interaction in real scenario + // In test environment, we verify it's set correctly + EXPECT_NE(testTray.notification_cb, nullptr); + + // Clear notification + testTray.notification_title = nullptr; + testTray.notification_text = nullptr; + testTray.notification_icon = nullptr; + testTray.notification_cb = nullptr; + tray_update(&testTray); +} + +TEST_F(TrayTest, TestTooltipUpdate) { + int initResult = tray_init(&testTray); + trayRunning = (initResult == 0); + ASSERT_EQ(initResult, 0); + + // Test initial tooltip + EXPECT_STREQ(testTray.tooltip, "TestTray"); + tray_loop(1); + WaitForTrayReady(); + EXPECT_TRUE(captureScreenshot("tray_tooltip_initial")); + + // Update tooltip + testTray.tooltip = "Updated Tooltip Text"; + tray_update(&testTray); + EXPECT_STREQ(testTray.tooltip, "Updated Tooltip Text"); + tray_loop(1); + WaitForTrayReady(); + EXPECT_TRUE(captureScreenshot("tray_tooltip_updated")); + + // Restore original tooltip + testTray.tooltip = "TestTray"; + tray_update(&testTray); +} + +TEST_F(TrayTest, TestMenuItemContext) { + static int contextValue = 42; + static bool contextCallbackInvoked = false; + + auto context_callback = [](struct tray_menu *item) { + if (item->context != nullptr) { + int *value = static_cast(item->context); + contextCallbackInvoked = (*value == 42); + } + }; + + // Create menu with context + struct tray_menu context_menu[] = { + {.text = "Context Item", .cb = context_callback, .context = &contextValue}, + {.text = nullptr} + }; + + testTray.menu = context_menu; + + int initResult = tray_init(&testTray); + trayRunning = (initResult == 0); + ASSERT_EQ(initResult, 0); + + // Verify context is set + EXPECT_EQ(testTray.menu[0].context, &contextValue); + + // Invoke callback with context + testTray.menu[0].cb(&testTray.menu[0]); + EXPECT_TRUE(contextCallbackInvoked); + + tray_loop(1); + WaitForTrayReady(); + EXPECT_TRUE(captureScreenshot("tray_menu_with_context")); + + // Restore original menu + testTray.menu = submenu; +} + +TEST_F(TrayTest, TestCheckboxStates) { + int initResult = tray_init(&testTray); + trayRunning = (initResult == 0); + ASSERT_EQ(initResult, 0); + + // Test checkbox item + EXPECT_EQ(testTray.menu[1].checkbox, 1); + EXPECT_EQ(testTray.menu[1].checked, 1); + tray_loop(1); + WaitForTrayReady(); + EXPECT_TRUE(captureScreenshot("tray_checkbox_checked")); + + // Toggle checkbox + testTray.menu[1].checked = 0; + tray_update(&testTray); + tray_loop(1); + WaitForTrayReady(); + EXPECT_TRUE(captureScreenshot("tray_checkbox_unchecked")); + + // Toggle back + testTray.menu[1].checked = 1; + tray_update(&testTray); +} + +TEST_F(TrayTest, TestMultipleIconUpdates) { + int initResult = tray_init(&testTray); + trayRunning = (initResult == 0); + ASSERT_EQ(initResult, 0); + + // Capture initial icon + WaitForTrayReady(); + EXPECT_TRUE(captureScreenshot("tray_icon_state1")); + + // Update icon multiple times + testTray.icon = TRAY_ICON2; + tray_update(&testTray); + WaitForTrayReady(); + EXPECT_TRUE(captureScreenshot("tray_icon_state2")); + + testTray.icon = TRAY_ICON1; + tray_update(&testTray); + WaitForTrayReady(); + EXPECT_TRUE(captureScreenshot("tray_icon_state3")); +} + +TEST_F(TrayTest, TestCompleteMenuHierarchy) { + int initResult = tray_init(&testTray); + trayRunning = (initResult == 0); + ASSERT_EQ(initResult, 0); + + // Verify complete menu structure + int menuCount = 0; + for (struct tray_menu *m = testTray.menu; m->text != nullptr; m++) { + menuCount++; + } + EXPECT_EQ(menuCount, 7); // Hello, Checked, Disabled, Sep, SubMenu, Sep, Quit + + // Verify all nested submenus + ASSERT_NE(testTray.menu[4].submenu, nullptr); + ASSERT_NE(testTray.menu[4].submenu[0].submenu, nullptr); + ASSERT_NE(testTray.menu[4].submenu[1].submenu, nullptr); + + tray_loop(1); + WaitForTrayReady(); + EXPECT_TRUE(captureScreenshot("tray_complete_menu_hierarchy")); +} + +TEST_F(TrayTest, TestIconPathArray) { +#if defined(TRAY_WINAPI) + // Test icon path array caching (Windows-specific feature) + // Allocate memory for tray struct with flexible array member + const size_t icon_count = 2; + struct tray *iconCacheTray = (struct tray *) malloc( + sizeof(struct tray) + icon_count * sizeof(const char *) + ); + ASSERT_NE(iconCacheTray, nullptr); + + // Initialize the tray structure + iconCacheTray->icon = TRAY_ICON1; + iconCacheTray->tooltip = "Icon Cache Test"; + iconCacheTray->notification_icon = nullptr; + iconCacheTray->notification_text = nullptr; + iconCacheTray->notification_title = nullptr; + iconCacheTray->notification_cb = nullptr; + iconCacheTray->menu = submenu; + *const_cast(&iconCacheTray->iconPathCount) = icon_count; + *const_cast(&iconCacheTray->allIconPaths[0]) = TRAY_ICON1; + *const_cast(&iconCacheTray->allIconPaths[1]) = TRAY_ICON2; + + int initResult = tray_init(iconCacheTray); + trayRunning = (initResult == 0); + ASSERT_EQ(initResult, 0); + + // Verify initial icon + EXPECT_EQ(iconCacheTray->icon, TRAY_ICON1); + tray_loop(1); + WaitForTrayReady(); + EXPECT_TRUE(captureScreenshot("tray_icon_cache_initial")); + + // Switch to cached icon + iconCacheTray->icon = TRAY_ICON2; + tray_update(iconCacheTray); + tray_loop(1); + WaitForTrayReady(); + EXPECT_TRUE(captureScreenshot("tray_icon_cache_updated")); + free(iconCacheTray); +#else + // On non-Windows platforms, just test basic icon switching + int initResult = tray_init(&testTray); + trayRunning = (initResult == 0); + ASSERT_EQ(initResult, 0); + + EXPECT_EQ(testTray.icon, TRAY_ICON1); + tray_loop(1); + WaitForTrayReady(); + EXPECT_TRUE(captureScreenshot("tray_icon_cache_initial")); + + testTray.icon = TRAY_ICON2; + tray_update(&testTray); + tray_loop(1); + WaitForTrayReady(); + EXPECT_TRUE(captureScreenshot("tray_icon_cache_updated")); +#endif +} + +TEST_F(TrayTest, TestQuitCallback) { + int initResult = tray_init(&testTray); + trayRunning = (initResult == 0); + ASSERT_EQ(initResult, 0); + + // Verify quit callback exists + ASSERT_NE(testTray.menu[6].cb, nullptr); + EXPECT_STREQ(testTray.menu[6].text, "Quit"); + + tray_loop(1); + WaitForTrayReady(); + EXPECT_TRUE(captureScreenshot("tray_before_quit")); + + // Note: Actually calling quit_cb would terminate the tray, + // which is tested separately in TestTrayExit +} + +TEST_F(TrayTest, TestTrayShowMenu) { + int initResult = tray_init(&testTray); + trayRunning = (initResult == 0); + ASSERT_EQ(initResult, 0); + + // Start a thread to capture screenshot and exit + std::thread capture_thread([this]() { + std::this_thread::sleep_for(std::chrono::milliseconds(500)); + WaitForTrayReady(); + EXPECT_TRUE(captureScreenshot("tray_menu_shown")); +#if defined(TRAY_WINAPI) + // Cancel the menu + PostMessage(tray_get_hwnd(), WM_CANCELMODE, 0, 0); + // Wait for menu to close + std::this_thread::sleep_for(std::chrono::milliseconds(100)); +#elif defined(TRAY_APPKIT) + // Simulate ESC key to dismiss menu + CGEventRef event = CGEventCreateKeyboardEvent(NULL, kVK_Escape, true); + CGEventPost(kCGHIDEventTap, event); + CFRelease(event); + CGEventRef event2 = CGEventCreateKeyboardEvent(NULL, kVK_Escape, false); + CGEventPost(kCGHIDEventTap, event2); + CFRelease(event2); + // Wait for menu to close + std::this_thread::sleep_for(std::chrono::milliseconds(100)); +#endif + // Exit the tray + tray_exit(); + }); + + // Show the menu programmatically + tray_show_menu(); + + tray_loop(1); + + capture_thread.join(); } TEST_F(TrayTest, TestTrayExit) { tray_exit(); - // TODO: Check the state after tray_exit } From 8b687ae98e66cce613e8ea51ef8d64533ada85e3 Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Thu, 8 Jan 2026 18:12:53 -0500 Subject: [PATCH 02/19] Improve tray menu popup handling on Linux Adds logic to create an invisible toplevel window as an anchor for the tray menu, ensuring compatibility with AppIndicator and headless environments. Uses modern GTK API when available and falls back to legacy methods as needed. --- src/tray_linux.c | 43 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/src/tray_linux.c b/src/tray_linux.c index 3efc7eb..b40b6c8 100644 --- a/src/tray_linux.c +++ b/src/tray_linux.c @@ -173,7 +173,48 @@ void tray_update(struct tray *tray) { void tray_show_menu(void) { if (current_menu != NULL) { - gtk_menu_popup(current_menu, NULL, NULL, NULL, NULL, 0, gtk_get_current_event_time()); + // For AppIndicator, we need to show the menu manually since the indicator + // is managed by the system. In headless environments, we need to create + // a proper toplevel window for the menu to attach to. + + // Create an invisible toplevel window as an anchor + GtkWidget *anchor_window = gtk_window_new(GTK_WINDOW_TOPLEVEL); + if (anchor_window != NULL) { + gtk_window_set_type_hint(GTK_WINDOW(anchor_window), GDK_WINDOW_TYPE_HINT_POPUP_MENU); + gtk_window_set_decorated(GTK_WINDOW(anchor_window), FALSE); + gtk_window_set_skip_taskbar_hint(GTK_WINDOW(anchor_window), TRUE); + gtk_window_set_skip_pager_hint(GTK_WINDOW(anchor_window), TRUE); + gtk_window_move(GTK_WINDOW(anchor_window), 100, 100); + gtk_window_resize(GTK_WINDOW(anchor_window), 1, 1); + gtk_widget_show(anchor_window); + + // Give the window time to appear + while (gtk_events_pending()) { + gtk_main_iteration(); + } + + if (gtk_check_version(3, 22, 0) == NULL) { + // GTK 3.22+ - use modern API with the anchor window + GdkWindow *gdk_window = gtk_widget_get_window(anchor_window); + if (gdk_window != NULL) { + GdkRectangle rect = {0, 0, 1, 1}; + gtk_menu_popup_at_rect(current_menu, gdk_window, &rect, + GDK_GRAVITY_NORTH_WEST, GDK_GRAVITY_NORTH_WEST, NULL); + } else { + // Fallback + gtk_menu_popup(current_menu, NULL, NULL, NULL, NULL, 0, gtk_get_current_event_time()); + } + } else { + // Older GTK - use deprecated API + gtk_menu_popup(current_menu, NULL, NULL, NULL, NULL, 0, gtk_get_current_event_time()); + } + + // Note: We don't destroy anchor_window here as the menu needs it to stay visible + // It will be cleaned up when the application exits + } else { + // Fallback if window creation fails + gtk_menu_popup(current_menu, NULL, NULL, NULL, NULL, 0, gtk_get_current_event_time()); + } } } From 08126ec38207a0693aaf5ecf3e45cbd87539d10b Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Thu, 8 Jan 2026 19:15:05 -0500 Subject: [PATCH 03/19] Add tooltip support to tray on macOS Tray items on macOS now display a tooltip if the 'tooltip' field is set. This improves user experience by providing additional context when hovering over the tray icon. --- src/tray_darwin.m | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/tray_darwin.m b/src/tray_darwin.m index b376c2d..7b4908a 100644 --- a/src/tray_darwin.m +++ b/src/tray_darwin.m @@ -119,6 +119,11 @@ void tray_update(struct tray *tray) { [image setSize:NSMakeSize(16, 16)]; statusItem.button.image = image; [statusItem setMenu:_tray_menu(tray->menu)]; + + // Set tooltip if provided + if (tray->tooltip != NULL) { + statusItem.button.toolTip = [NSString stringWithUTF8String:tray->tooltip]; + } } void tray_show_menu(void) { From 29af496f258166d41f8f113e3cc2861d93c73f0c Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Thu, 8 Jan 2026 19:23:48 -0500 Subject: [PATCH 04/19] Remove redundant tray_loop calls from tray tests Eliminated unnecessary tray_loop(1) calls in unit tests for tooltip, checkbox, and icon updates. WaitForTrayReady is sufficient for synchronization, simplifying the test logic. --- tests/unit/test_tray.cpp | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tests/unit/test_tray.cpp b/tests/unit/test_tray.cpp index 509db28..fbbcf63 100644 --- a/tests/unit/test_tray.cpp +++ b/tests/unit/test_tray.cpp @@ -388,7 +388,6 @@ TEST_F(TrayTest, TestTooltipUpdate) { // Test initial tooltip EXPECT_STREQ(testTray.tooltip, "TestTray"); - tray_loop(1); WaitForTrayReady(); EXPECT_TRUE(captureScreenshot("tray_tooltip_initial")); @@ -396,7 +395,6 @@ TEST_F(TrayTest, TestTooltipUpdate) { testTray.tooltip = "Updated Tooltip Text"; tray_update(&testTray); EXPECT_STREQ(testTray.tooltip, "Updated Tooltip Text"); - tray_loop(1); WaitForTrayReady(); EXPECT_TRUE(captureScreenshot("tray_tooltip_updated")); @@ -451,14 +449,12 @@ TEST_F(TrayTest, TestCheckboxStates) { // Test checkbox item EXPECT_EQ(testTray.menu[1].checkbox, 1); EXPECT_EQ(testTray.menu[1].checked, 1); - tray_loop(1); WaitForTrayReady(); EXPECT_TRUE(captureScreenshot("tray_checkbox_checked")); // Toggle checkbox testTray.menu[1].checked = 0; tray_update(&testTray); - tray_loop(1); WaitForTrayReady(); EXPECT_TRUE(captureScreenshot("tray_checkbox_unchecked")); @@ -538,14 +534,12 @@ TEST_F(TrayTest, TestIconPathArray) { // Verify initial icon EXPECT_EQ(iconCacheTray->icon, TRAY_ICON1); - tray_loop(1); WaitForTrayReady(); EXPECT_TRUE(captureScreenshot("tray_icon_cache_initial")); // Switch to cached icon iconCacheTray->icon = TRAY_ICON2; tray_update(iconCacheTray); - tray_loop(1); WaitForTrayReady(); EXPECT_TRUE(captureScreenshot("tray_icon_cache_updated")); free(iconCacheTray); @@ -556,13 +550,11 @@ TEST_F(TrayTest, TestIconPathArray) { ASSERT_EQ(initResult, 0); EXPECT_EQ(testTray.icon, TRAY_ICON1); - tray_loop(1); WaitForTrayReady(); EXPECT_TRUE(captureScreenshot("tray_icon_cache_initial")); testTray.icon = TRAY_ICON2; tray_update(&testTray); - tray_loop(1); WaitForTrayReady(); EXPECT_TRUE(captureScreenshot("tray_icon_cache_updated")); #endif From c362b5a6e387862a7225f3fbe9315ff3f37325da Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Thu, 8 Jan 2026 19:54:22 -0500 Subject: [PATCH 05/19] Ensure status item is removed on main thread in tray_exit Updated tray_exit to remove the status item from the status bar using dispatch_async on the main thread, ensuring thread safety for NSStatusBar operations. --- src/tray_darwin.m | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/tray_darwin.m b/src/tray_darwin.m index 7b4908a..b4f3761 100644 --- a/src/tray_darwin.m +++ b/src/tray_darwin.m @@ -131,6 +131,18 @@ void tray_show_menu(void) { } void tray_exit(void) { + // Remove the status item from the status bar on the main thread + // NSStatusBar operations must be performed on the main thread + if (statusItem != nil) { + dispatch_async(dispatch_get_main_queue(), ^{ + if (statusItem != nil) { + [statusBar removeStatusItem:statusItem]; + statusItem = nil; + } + }); + } + + // Post exit event NSEvent *event = [NSEvent otherEventWithType:NSEventTypeApplicationDefined location:NSMakePoint(0, 0) modifierFlags:0 From 6bf27bd42ca83e64dec96afcdaeb730fe754b3c6 Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Thu, 8 Jan 2026 20:19:20 -0500 Subject: [PATCH 06/19] Remove blocking tray_loop calls from tray tests Replaces or removes calls to tray_loop(1) in unit tests to prevent hanging during test execution. The TestTrayLoop now uses tray_loop(0) for non-blocking behavior, and other tests no longer invoke tray_loop, improving test reliability and speed. --- tests/unit/test_tray.cpp | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/tests/unit/test_tray.cpp b/tests/unit/test_tray.cpp index fbbcf63..eee7ab5 100644 --- a/tests/unit/test_tray.cpp +++ b/tests/unit/test_tray.cpp @@ -190,7 +190,8 @@ TEST_F(TrayTest, TestTrayLoop) { int initResult = tray_init(&testTray); trayRunning = (initResult == 0); ASSERT_EQ(initResult, 0); - int result = tray_loop(1); + // Test non-blocking loop (blocking=0) since blocking would hang without events + int result = tray_loop(0); EXPECT_EQ(result, 0); WaitForTrayReady(); EXPECT_TRUE(captureScreenshot("tray_loop_iteration")); @@ -237,7 +238,6 @@ TEST_F(TrayTest, TestMenuItemCallback) { // Test hello callback - it should work without crashing ASSERT_NE(testTray.menu[0].cb, nullptr); testTray.menu[0].cb(&testTray.menu[0]); - tray_loop(1); WaitForTrayReady(); EXPECT_TRUE(captureScreenshot("tray_menu_callback_hello")); } @@ -250,7 +250,6 @@ TEST_F(TrayTest, TestDisabledMenuItem) { // Verify disabled menu item EXPECT_EQ(testTray.menu[2].disabled, 1); EXPECT_STREQ(testTray.menu[2].text, "Disabled"); - tray_loop(1); WaitForTrayReady(); EXPECT_TRUE(captureScreenshot("tray_menu_disabled_item")); } @@ -263,7 +262,6 @@ TEST_F(TrayTest, TestMenuSeparator) { // Verify separator exists EXPECT_STREQ(testTray.menu[3].text, "-"); EXPECT_EQ(testTray.menu[3].cb, nullptr); - tray_loop(1); WaitForTrayReady(); EXPECT_TRUE(captureScreenshot("tray_menu_with_separator")); } @@ -282,7 +280,6 @@ TEST_F(TrayTest, TestSubmenuStructure) { ASSERT_NE(testTray.menu[4].submenu[0].submenu, nullptr); EXPECT_STREQ(testTray.menu[4].submenu[0].submenu[0].text, "7"); - tray_loop(1); WaitForTrayReady(); EXPECT_TRUE(captureScreenshot("tray_submenu_structure")); } @@ -295,7 +292,6 @@ TEST_F(TrayTest, TestSubmenuCallback) { // Test submenu callback ASSERT_NE(testTray.menu[4].submenu[0].submenu[0].cb, nullptr); testTray.menu[4].submenu[0].submenu[0].cb(&testTray.menu[4].submenu[0].submenu[0]); - tray_loop(1); WaitForTrayReady(); EXPECT_TRUE(captureScreenshot("tray_submenu_callback_executed")); } @@ -323,7 +319,6 @@ TEST_F(TrayTest, TestNotificationDisplay) { testTray.notification_icon = TRAY_ICON1; tray_update(&testTray); - tray_loop(1); WaitForTrayReady(); EXPECT_TRUE(captureScreenshot("tray_notification_displayed")); @@ -364,7 +359,6 @@ TEST_F(TrayTest, TestNotificationCallback) { testTray.notification_cb = notification_callback; tray_update(&testTray); - tray_loop(1); WaitForTrayReady(); EXPECT_TRUE(captureScreenshot("tray_notification_with_callback")); @@ -433,7 +427,6 @@ TEST_F(TrayTest, TestMenuItemContext) { testTray.menu[0].cb(&testTray.menu[0]); EXPECT_TRUE(contextCallbackInvoked); - tray_loop(1); WaitForTrayReady(); EXPECT_TRUE(captureScreenshot("tray_menu_with_context")); @@ -501,7 +494,6 @@ TEST_F(TrayTest, TestCompleteMenuHierarchy) { ASSERT_NE(testTray.menu[4].submenu[0].submenu, nullptr); ASSERT_NE(testTray.menu[4].submenu[1].submenu, nullptr); - tray_loop(1); WaitForTrayReady(); EXPECT_TRUE(captureScreenshot("tray_complete_menu_hierarchy")); } @@ -569,7 +561,6 @@ TEST_F(TrayTest, TestQuitCallback) { ASSERT_NE(testTray.menu[6].cb, nullptr); EXPECT_STREQ(testTray.menu[6].text, "Quit"); - tray_loop(1); WaitForTrayReady(); EXPECT_TRUE(captureScreenshot("tray_before_quit")); From 6a72daac5580a140b744f7a0717553c8f9742808 Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Thu, 8 Jan 2026 20:24:46 -0500 Subject: [PATCH 07/19] Fix status item removal to ensure main thread execution Updated tray_exit to check if the current thread is the main thread before removing the status item. If not on the main thread, the removal is dispatched synchronously to the main thread to ensure thread safety. --- src/tray_darwin.m | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/tray_darwin.m b/src/tray_darwin.m index b4f3761..2cebb17 100644 --- a/src/tray_darwin.m +++ b/src/tray_darwin.m @@ -134,12 +134,19 @@ void tray_exit(void) { // Remove the status item from the status bar on the main thread // NSStatusBar operations must be performed on the main thread if (statusItem != nil) { - dispatch_async(dispatch_get_main_queue(), ^{ - if (statusItem != nil) { - [statusBar removeStatusItem:statusItem]; - statusItem = nil; - } - }); + if ([NSThread isMainThread]) { + // Already on main thread, remove directly + [statusBar removeStatusItem:statusItem]; + statusItem = nil; + } else { + // On background thread, dispatch synchronously to main thread + dispatch_sync(dispatch_get_main_queue(), ^{ + if (statusItem != nil) { + [statusBar removeStatusItem:statusItem]; + statusItem = nil; + } + }); + } } // Post exit event From a9e083acf6f151db148cb365d3f21bedfdc06c40 Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Thu, 8 Jan 2026 20:35:45 -0500 Subject: [PATCH 08/19] Improve WaitForTrayReady for macOS and Linux Enhanced the WaitForTrayReady method to handle macOS (AppKit) by adding a delay, ensuring the tray icon appears before screenshots. Updated comments for clarity and maintained Linux (AppIndicator) event processing. --- tests/unit/test_tray.cpp | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/tests/unit/test_tray.cpp b/tests/unit/test_tray.cpp index eee7ab5..d04ad23 100644 --- a/tests/unit/test_tray.cpp +++ b/tests/unit/test_tray.cpp @@ -130,17 +130,29 @@ class TrayTest: public BaseTest { trayRunning = false; } - // Process pending GTK events to allow AppIndicator to register + // Process pending events to allow tray icon to appear // Call this ONLY before screenshots to ensure the icon is visible void WaitForTrayReady() { #if defined(TRAY_APPINDICATOR) - // On Linux with AppIndicator, process GTK events to allow D-Bus registration - // This ensures the icon actually appears in the system tray before screenshots - // Use shorter iterations to avoid interfering with event loop state + // On Linux: process GTK events to allow AppIndicator D-Bus registration for (int i = 0; i < 100; i++) { tray_loop(0); // Non-blocking - process pending events std::this_thread::sleep_for(std::chrono::milliseconds(5)); } +#elif defined(TRAY_APPKIT) + // On macOS: process events if on main thread, otherwise just sleep + // NSApp event loop must only be called from main thread + static std::thread::id main_thread_id = std::this_thread::get_id(); + if (std::this_thread::get_id() == main_thread_id) { + // Main thread - safe to call tray_loop + for (int i = 0; i < 100; i++) { + tray_loop(0); // Non-blocking - process pending events + std::this_thread::sleep_for(std::chrono::milliseconds(5)); + } + } else { + // Background thread - just wait longer + std::this_thread::sleep_for(std::chrono::milliseconds(1000)); + } #endif } }; From 50f1415ff66a64c24a91fbbaa099facaab723967 Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Thu, 8 Jan 2026 21:22:55 -0500 Subject: [PATCH 09/19] Add macOS screen recording permission fix in CI Introduces a step to grant screen recording permissions on macOS runners by modifying the TCC database, preventing popup dialogs during CI runs. This ensures smoother automated workflows and addresses issues with screen capture access in GitHub Actions. --- .github/workflows/ci.yml | 71 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3d5b174..d3cff7b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -73,6 +73,77 @@ jobs: ninja \ node + - name: Fix macOS screen recording permissions + if: runner.os == 'macOS' + run: | + # Grant screen recording permissions to prevent popup dialogs in CI + # This modifies the TCC (Transparency, Consent, and Control) database + + # https://apple.stackexchange.com/questions/362865/macos-list-apps-authorized-for-full-disk-access + # https://github.com/actions/runner-images/issues/9529 + # https://github.com/actions/runner-images/pull/9530 + + # Get macOS version + os_version=$(sw_vers -productVersion | cut -d '.' -f 1) + echo "macOS version: $os_version" + + # function to execute sql query for each value + function execute_sql_query { + local value=$1 + local dbPath=$2 + echo "Executing SQL query for value: $value" + sudo sqlite3 "$dbPath" "INSERT OR IGNORE INTO access VALUES($value);" + } + + # Find all provisioner paths and store them in an array + declare -a app_paths=() + while IFS= read -r line; do + app_paths+=("$line") + done < <(sudo find /opt /usr -name bash 2>/dev/null) + echo "Provisioner paths: ${app_paths[@]}" + + # Create an empty array + declare -a values=() + + # Loop through the provisioner paths and add them to the values array + for p_path in "${app_paths[@]}"; do + # Adjust the service name and other parameters as needed + values+=("'kTCCServiceAccessibility','${p_path}',1,2,4,1,NULL,NULL,0,'UNUSED',NULL,NULL,1592919552") + values+=("'kTCCServiceScreenCapture','${p_path}',1,2,4,1,NULL,NULL,0,'UNUSED',NULL,0,1687786159") + done + echo "Values: ${values[@]}" + + # Adjust for Sonoma (macOS 14+) which has extra columns + if [[ "$os_version" -ge 14 ]]; then + echo "Adjusting for macOS Sonoma or later (extra TCC columns)" + # TCC access table in Sonoma has extra 4 columns: pid, pid_version, boot_uuid, last_reminded + for i in "${!values[@]}"; do + values[$i]="${values[$i]},NULL,NULL,'UNUSED',${values[$i]##*,}" + done + fi + + # System and user TCC databases + dbPaths=( + "/Library/Application Support/com.apple.TCC/TCC.db" + "$HOME/Library/Application Support/com.apple.TCC/TCC.db" + ) + + # Execute SQL queries + for value in "${values[@]}"; do + for dbPath in "${dbPaths[@]}"; do + echo "Column names for $dbPath" + echo "-------------------" + sudo sqlite3 "$dbPath" "PRAGMA table_info(access);" + echo "Current permissions for $dbPath" + echo "-------------------" + sudo sqlite3 "$dbPath" "SELECT * FROM access WHERE service='kTCCServiceScreenCapture';" + execute_sql_query "$value" "$dbPath" + echo "Updated permissions for $dbPath" + echo "-------------------" + sudo sqlite3 "$dbPath" "SELECT * FROM access WHERE service='kTCCServiceScreenCapture';" + done + done + - name: Setup Dependencies Windows if: runner.os == 'Windows' uses: msys2/setup-msys2@4f806de0a5a7294ffabaff804b38a9b435a73bda # v2.30.0 From 80c19af01cf45cd99d8530d910ba4c376d196cd2 Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Fri, 9 Jan 2026 22:19:39 -0500 Subject: [PATCH 10/19] Add appindicator_type to CI matrix and setup Introduces an appindicator_type variable to the CI matrix for distinguishing between 'ayatana' and 'legacy' appindicator packages. Passes this variable to the setup_virtual_desktop action to allow for more flexible environment configuration. --- .github/workflows/ci.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d3cff7b..3a56573 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,9 +29,11 @@ jobs: shell: "bash" - os: ubuntu-latest appindicator: "libayatana-appindicator3-dev" + appindicator_type: "ayatana" shell: "bash" - os: ubuntu-latest appindicator: "libappindicator3-dev" + appindicator_type: "legacy" shell: "bash" - os: windows-latest shell: "msys2 {0}" @@ -59,6 +61,7 @@ jobs: if: runner.os == 'Linux' uses: LizardByte/actions/actions/setup_virtual_desktop@feat/actions/linux-display # todo: pin version with: + appindicator-version: ${{ matrix.appindicator_type }} environment: mate - name: Setup Dependencies macOS From 6167a9e7ee9befdd7050c807ab6decb25bc4a610 Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Wed, 25 Feb 2026 19:01:12 -0500 Subject: [PATCH 11/19] Enhance CI D-Bus checks; fix tests and consts Pin setup_virtual_desktop action to a specific commit and add extensive D-Bus environment checks in the CI test step: print DBUS_SESSION_BUS_ADDRESS, attempt fallback detection from a dbus-daemon process, verify the D-Bus session responds, and warn if AppIndicator service is missing before running tests. In tests, ensure BaseTest::SetUp() is called in the platform-specific SetUp overrides to run common setup logic. Clean up includes and tighten const-correctness in screenshot_utils.cpp (remove unused headers and make local variables const). These changes aim to reduce flaky Linux AppIndicator test failures by ensuring a working D-Bus session and to improve code quality in tests. --- .github/workflows/ci.yml | 51 ++++++++++++++++++++++++++++++++++++-- src/tray_linux.c | 3 +-- tests/conftest.cpp | 2 ++ tests/screenshot_utils.cpp | 8 +++--- 4 files changed, 55 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3a56573..10d7977 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -59,7 +59,8 @@ jobs: - name: Setup virtual desktop if: runner.os == 'Linux' - uses: LizardByte/actions/actions/setup_virtual_desktop@feat/actions/linux-display # todo: pin version + uses: + LizardByte/actions/actions/setup_virtual_desktop@6be4ea62064b64957aa880252fe353cd65bcdef7 # v2026.214.14019 with: appindicator-version: ${{ matrix.appindicator_type }} environment: mate @@ -242,7 +243,53 @@ jobs: # TODO: tests randomly hang on Linux, https://github.com/LizardByte/tray/issues/45 timeout-minutes: 3 working-directory: build/tests - run: ./test_tray --gtest_color=yes --gtest_output=xml:test_results.xml + run: | + echo "::group::D-Bus Environment Check" + echo "DBUS_SESSION_BUS_ADDRESS: ${DBUS_SESSION_BUS_ADDRESS:-NOT SET}" + + # Ensure D-Bus session address is set for AppIndicator tests on Linux + if [ "${{ runner.os }}" = "Linux" ] && [ -z "$DBUS_SESSION_BUS_ADDRESS" ]; then + echo "::error::DBUS_SESSION_BUS_ADDRESS is not set! This should have been set by setup_virtual_desktop." + echo "Attempting to detect as fallback..." + DBUS_PID=$(pgrep -u $(id -u) dbus-daemon | head -n 1) + if [ -n "$DBUS_PID" ]; then + DBUS_ADDR=$(grep -z DBUS_SESSION_BUS_ADDRESS /proc/$DBUS_PID/environ | tr '\0' '\n' | cut -d= -f2-) + if [ -n "$DBUS_ADDR" ]; then + echo "Setting DBUS_SESSION_BUS_ADDRESS=$DBUS_ADDR" + export DBUS_SESSION_BUS_ADDRESS="$DBUS_ADDR" + else + echo "::error::Could not detect D-Bus session address" + exit 1 + fi + else + echo "::error::No dbus-daemon process found" + exit 1 + fi + else + echo "✓ DBUS_SESSION_BUS_ADDRESS is set" + fi + + # Verify D-Bus is responding + if [ "${{ runner.os }}" = "Linux" ]; then + if dbus-send --session --dest=org.freedesktop.DBus --print-reply /org/freedesktop/DBus \ + org.freedesktop.DBus.ListNames >/dev/null 2>&1; then + echo "✓ D-Bus session is responding" + else + echo "::error::D-Bus session is not responding" + exit 1 + fi + + # Check if indicator service is registered + if dbus-send --session --dest=org.freedesktop.DBus --print-reply /org/freedesktop/DBus \ + org.freedesktop.DBus.ListNames 2>/dev/null | grep -q "com.canonical.indicator.application"; then + echo "✓ AppIndicator service is registered on D-Bus" + else + echo "::warning::AppIndicator service is NOT registered on D-Bus" + fi + fi + echo "::endgroup::" + + ./test_tray --gtest_color=yes --gtest_output=xml:test_results.xml - name: Upload screenshots if: >- diff --git a/src/tray_linux.c b/src/tray_linux.c index b40b6c8..2ec5a2a 100644 --- a/src/tray_linux.c +++ b/src/tray_linux.c @@ -198,8 +198,7 @@ void tray_show_menu(void) { GdkWindow *gdk_window = gtk_widget_get_window(anchor_window); if (gdk_window != NULL) { GdkRectangle rect = {0, 0, 1, 1}; - gtk_menu_popup_at_rect(current_menu, gdk_window, &rect, - GDK_GRAVITY_NORTH_WEST, GDK_GRAVITY_NORTH_WEST, NULL); + gtk_menu_popup_at_rect(current_menu, gdk_window, &rect, GDK_GRAVITY_NORTH_WEST, GDK_GRAVITY_NORTH_WEST, NULL); } else { // Fallback gtk_menu_popup(current_menu, NULL, NULL, NULL, NULL, 0, gtk_get_current_event_time()); diff --git a/tests/conftest.cpp b/tests/conftest.cpp index 64ad934..e5f04c4 100644 --- a/tests/conftest.cpp +++ b/tests/conftest.cpp @@ -185,6 +185,7 @@ class LinuxTest: public BaseTest { #ifndef __linux__ GTEST_SKIP_("Skipping, this test is for Linux only."); #endif + BaseTest::SetUp(); } void TearDown() override { @@ -198,6 +199,7 @@ class MacOSTest: public BaseTest { #if !defined(__APPLE__) || !defined(__MACH__) GTEST_SKIP_("Skipping, this test is for macOS only."); #endif + BaseTest::SetUp(); } void TearDown() override { diff --git a/tests/screenshot_utils.cpp b/tests/screenshot_utils.cpp index cbed19c..1243d38 100644 --- a/tests/screenshot_utils.cpp +++ b/tests/screenshot_utils.cpp @@ -4,9 +4,7 @@ // standard includes #include #include -#include #include -#include #include #include #include @@ -27,11 +25,11 @@ namespace { std::filesystem::path g_outputRoot; std::string quote_shell_path(const std::filesystem::path &path) { - std::string input = path.string(); + const std::string input = path.string(); std::string output; output.reserve(input.size() + 2); output.push_back('"'); - for (char ch : input) { + for (const char ch : input) { if (ch == '"') { output.append("\\\""); } else { @@ -189,7 +187,7 @@ namespace screenshot { std::cerr << "PNG encoder CLSID not found" << std::endl; return false; } - std::wstring widePath = file.wstring(); + const std::wstring widePath = file.wstring(); if (bitmap.Save(widePath.c_str(), &pngClsid, nullptr) != Gdiplus::Ok) { std::cerr << "GDI+ failed to write " << file << std::endl; return false; From 825acdd021522cd371f863ff4ed072395ed1cd9e Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Wed, 25 Feb 2026 21:24:40 -0500 Subject: [PATCH 12/19] Simplify CI test step by removing D-Bus checks Remove verbose D-Bus environment detection, fallback logic, and verification steps from the CI test job. The workflow now runs ./test_tray --gtest_color=yes --gtest_output=xml:test_results.xml directly (working-directory: build/tests). This trims the CI step by deleting the DBUS_SESSION_BUS_ADDRESS checks, pgrep-/proc fallback, dbus-send responsiveness tests, and AppIndicator registration check that were previously logging and potentially causing hangs. --- .github/workflows/ci.yml | 48 +--------------------------------------- 1 file changed, 1 insertion(+), 47 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 10d7977..7cd5bc5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -243,53 +243,7 @@ jobs: # TODO: tests randomly hang on Linux, https://github.com/LizardByte/tray/issues/45 timeout-minutes: 3 working-directory: build/tests - run: | - echo "::group::D-Bus Environment Check" - echo "DBUS_SESSION_BUS_ADDRESS: ${DBUS_SESSION_BUS_ADDRESS:-NOT SET}" - - # Ensure D-Bus session address is set for AppIndicator tests on Linux - if [ "${{ runner.os }}" = "Linux" ] && [ -z "$DBUS_SESSION_BUS_ADDRESS" ]; then - echo "::error::DBUS_SESSION_BUS_ADDRESS is not set! This should have been set by setup_virtual_desktop." - echo "Attempting to detect as fallback..." - DBUS_PID=$(pgrep -u $(id -u) dbus-daemon | head -n 1) - if [ -n "$DBUS_PID" ]; then - DBUS_ADDR=$(grep -z DBUS_SESSION_BUS_ADDRESS /proc/$DBUS_PID/environ | tr '\0' '\n' | cut -d= -f2-) - if [ -n "$DBUS_ADDR" ]; then - echo "Setting DBUS_SESSION_BUS_ADDRESS=$DBUS_ADDR" - export DBUS_SESSION_BUS_ADDRESS="$DBUS_ADDR" - else - echo "::error::Could not detect D-Bus session address" - exit 1 - fi - else - echo "::error::No dbus-daemon process found" - exit 1 - fi - else - echo "✓ DBUS_SESSION_BUS_ADDRESS is set" - fi - - # Verify D-Bus is responding - if [ "${{ runner.os }}" = "Linux" ]; then - if dbus-send --session --dest=org.freedesktop.DBus --print-reply /org/freedesktop/DBus \ - org.freedesktop.DBus.ListNames >/dev/null 2>&1; then - echo "✓ D-Bus session is responding" - else - echo "::error::D-Bus session is not responding" - exit 1 - fi - - # Check if indicator service is registered - if dbus-send --session --dest=org.freedesktop.DBus --print-reply /org/freedesktop/DBus \ - org.freedesktop.DBus.ListNames 2>/dev/null | grep -q "com.canonical.indicator.application"; then - echo "✓ AppIndicator service is registered on D-Bus" - else - echo "::warning::AppIndicator service is NOT registered on D-Bus" - fi - fi - echo "::endgroup::" - - ./test_tray --gtest_color=yes --gtest_output=xml:test_results.xml + run: ./test_tray --gtest_color=yes --gtest_output=xml:test_results.xml - name: Upload screenshots if: >- From 7ca90cef10fbe93a992c307b84d7c78d69f20c1a Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Fri, 27 Feb 2026 16:07:04 -0500 Subject: [PATCH 13/19] Use virtual_desktop action in CI workflow Update the 'Setup virtual desktop' step in the CI workflow to use LizardByte/actions/actions/virtual_desktop@70bb8d3 (v2026.227.200013) instead of the older setup_virtual_desktop action. This switches to the newer action tag for the Linux virtual desktop setup while preserving the existing inputs and environment. --- .github/workflows/ci.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7cd5bc5..b7c656a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -59,8 +59,7 @@ jobs: - name: Setup virtual desktop if: runner.os == 'Linux' - uses: - LizardByte/actions/actions/setup_virtual_desktop@6be4ea62064b64957aa880252fe353cd65bcdef7 # v2026.214.14019 + uses: LizardByte/actions/actions/virtual_desktop@70bb8d394d1c92f6113aeec6ae9cc959a5763d15 # v2026.227.200013 with: appindicator-version: ${{ matrix.appindicator_type }} environment: mate From fc238f8641037c92e315e093aec6139b75dc2465 Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Sun, 22 Mar 2026 17:46:22 -0400 Subject: [PATCH 14/19] Refactor tray tests to replace screenshots/waits Remove brittle WaitForTrayReady() and inline EXPECT_TRUE(captureScreenshot(...)) calls across tray unit tests. Replace them with deterministic capture threads that open the tray/menu (tray_show_menu), sleep briefly, capture screenshots, and then programmatically close the menu (platform-specific: PostMessage WM_CANCELMODE on Windows, synthesize Escape on macOS) before calling tray_exit(). Adjust checkbox test to reinitialize the tray to exercise checked/unchecked captures. Overall this makes visual-capture steps explicit and less timing-dependent. --- tests/unit/test_tray.cpp | 130 ++++++++++++++++++++++----------------- 1 file changed, 74 insertions(+), 56 deletions(-) diff --git a/tests/unit/test_tray.cpp b/tests/unit/test_tray.cpp index d04ad23..6d21fc3 100644 --- a/tests/unit/test_tray.cpp +++ b/tests/unit/test_tray.cpp @@ -205,8 +205,6 @@ TEST_F(TrayTest, TestTrayLoop) { // Test non-blocking loop (blocking=0) since blocking would hang without events int result = tray_loop(0); EXPECT_EQ(result, 0); - WaitForTrayReady(); - EXPECT_TRUE(captureScreenshot("tray_loop_iteration")); } TEST_F(TrayTest, TestTrayUpdate) { @@ -220,8 +218,6 @@ TEST_F(TrayTest, TestTrayUpdate) { testTray.tooltip = "TestTray2"; tray_update(&testTray); EXPECT_EQ(testTray.icon, TRAY_ICON2); - WaitForTrayReady(); - EXPECT_TRUE(captureScreenshot("tray_icon_updated")); // put back the original values testTray.icon = TRAY_ICON1; @@ -238,8 +234,6 @@ TEST_F(TrayTest, TestToggleCallback) { bool initialCheckedState = testTray.menu[1].checked; toggle_cb(&testTray.menu[1]); EXPECT_EQ(testTray.menu[1].checked, !initialCheckedState); - WaitForTrayReady(); - EXPECT_TRUE(captureScreenshot("tray_menu_toggle")); } TEST_F(TrayTest, TestMenuItemCallback) { @@ -250,8 +244,6 @@ TEST_F(TrayTest, TestMenuItemCallback) { // Test hello callback - it should work without crashing ASSERT_NE(testTray.menu[0].cb, nullptr); testTray.menu[0].cb(&testTray.menu[0]); - WaitForTrayReady(); - EXPECT_TRUE(captureScreenshot("tray_menu_callback_hello")); } TEST_F(TrayTest, TestDisabledMenuItem) { @@ -262,8 +254,6 @@ TEST_F(TrayTest, TestDisabledMenuItem) { // Verify disabled menu item EXPECT_EQ(testTray.menu[2].disabled, 1); EXPECT_STREQ(testTray.menu[2].text, "Disabled"); - WaitForTrayReady(); - EXPECT_TRUE(captureScreenshot("tray_menu_disabled_item")); } TEST_F(TrayTest, TestMenuSeparator) { @@ -274,8 +264,6 @@ TEST_F(TrayTest, TestMenuSeparator) { // Verify separator exists EXPECT_STREQ(testTray.menu[3].text, "-"); EXPECT_EQ(testTray.menu[3].cb, nullptr); - WaitForTrayReady(); - EXPECT_TRUE(captureScreenshot("tray_menu_with_separator")); } TEST_F(TrayTest, TestSubmenuStructure) { @@ -292,8 +280,28 @@ TEST_F(TrayTest, TestSubmenuStructure) { ASSERT_NE(testTray.menu[4].submenu[0].submenu, nullptr); EXPECT_STREQ(testTray.menu[4].submenu[0].submenu[0].text, "7"); - WaitForTrayReady(); - EXPECT_TRUE(captureScreenshot("tray_submenu_structure")); + // Show the menu open to capture nested menus visually + std::thread capture_thread([this]() { + std::this_thread::sleep_for(std::chrono::milliseconds(500)); + EXPECT_TRUE(captureScreenshot("tray_menu_with_submenu")); +#if defined(TRAY_WINAPI) + PostMessage(tray_get_hwnd(), WM_CANCELMODE, 0, 0); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); +#elif defined(TRAY_APPKIT) + CGEventRef event = CGEventCreateKeyboardEvent(NULL, kVK_Escape, true); + CGEventPost(kCGHIDEventTap, event); + CFRelease(event); + CGEventRef event2 = CGEventCreateKeyboardEvent(NULL, kVK_Escape, false); + CGEventPost(kCGHIDEventTap, event2); + CFRelease(event2); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); +#endif + tray_exit(); + }); + + tray_show_menu(); + tray_loop(1); + capture_thread.join(); } TEST_F(TrayTest, TestSubmenuCallback) { @@ -304,8 +312,6 @@ TEST_F(TrayTest, TestSubmenuCallback) { // Test submenu callback ASSERT_NE(testTray.menu[4].submenu[0].submenu[0].cb, nullptr); testTray.menu[4].submenu[0].submenu[0].cb(&testTray.menu[4].submenu[0].submenu[0]); - WaitForTrayReady(); - EXPECT_TRUE(captureScreenshot("tray_submenu_callback_executed")); } TEST_F(TrayTest, TestNotificationDisplay) { @@ -372,9 +378,6 @@ TEST_F(TrayTest, TestNotificationCallback) { tray_update(&testTray); - WaitForTrayReady(); - EXPECT_TRUE(captureScreenshot("tray_notification_with_callback")); - // Note: callback would be invoked by user interaction in real scenario // In test environment, we verify it's set correctly EXPECT_NE(testTray.notification_cb, nullptr); @@ -394,15 +397,11 @@ TEST_F(TrayTest, TestTooltipUpdate) { // Test initial tooltip EXPECT_STREQ(testTray.tooltip, "TestTray"); - WaitForTrayReady(); - EXPECT_TRUE(captureScreenshot("tray_tooltip_initial")); // Update tooltip testTray.tooltip = "Updated Tooltip Text"; tray_update(&testTray); EXPECT_STREQ(testTray.tooltip, "Updated Tooltip Text"); - WaitForTrayReady(); - EXPECT_TRUE(captureScreenshot("tray_tooltip_updated")); // Restore original tooltip testTray.tooltip = "TestTray"; @@ -439,9 +438,6 @@ TEST_F(TrayTest, TestMenuItemContext) { testTray.menu[0].cb(&testTray.menu[0]); EXPECT_TRUE(contextCallbackInvoked); - WaitForTrayReady(); - EXPECT_TRUE(captureScreenshot("tray_menu_with_context")); - // Restore original menu testTray.menu = submenu; } @@ -451,21 +447,64 @@ TEST_F(TrayTest, TestCheckboxStates) { trayRunning = (initResult == 0); ASSERT_EQ(initResult, 0); - // Test checkbox item EXPECT_EQ(testTray.menu[1].checkbox, 1); EXPECT_EQ(testTray.menu[1].checked, 1); - WaitForTrayReady(); - EXPECT_TRUE(captureScreenshot("tray_checkbox_checked")); - // Toggle checkbox + // Show menu open with checkbox in checked state + std::thread capture_checked([this]() { + std::this_thread::sleep_for(std::chrono::milliseconds(500)); + EXPECT_TRUE(captureScreenshot("tray_menu_checkbox_checked")); +#if defined(TRAY_WINAPI) + PostMessage(tray_get_hwnd(), WM_CANCELMODE, 0, 0); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); +#elif defined(TRAY_APPKIT) + CGEventRef event = CGEventCreateKeyboardEvent(NULL, kVK_Escape, true); + CGEventPost(kCGHIDEventTap, event); + CFRelease(event); + CGEventRef event2 = CGEventCreateKeyboardEvent(NULL, kVK_Escape, false); + CGEventPost(kCGHIDEventTap, event2); + CFRelease(event2); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); +#endif + tray_exit(); + }); + + tray_show_menu(); + tray_loop(1); + capture_checked.join(); + + // Re-initialize tray with checkbox unchecked + trayRunning = false; testTray.menu[1].checked = 0; - tray_update(&testTray); - WaitForTrayReady(); - EXPECT_TRUE(captureScreenshot("tray_checkbox_unchecked")); + initResult = tray_init(&testTray); + trayRunning = (initResult == 0); + ASSERT_EQ(initResult, 0); + + // Show menu open with checkbox in unchecked state + std::thread capture_unchecked([this]() { + std::this_thread::sleep_for(std::chrono::milliseconds(500)); + EXPECT_TRUE(captureScreenshot("tray_menu_checkbox_unchecked")); +#if defined(TRAY_WINAPI) + PostMessage(tray_get_hwnd(), WM_CANCELMODE, 0, 0); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); +#elif defined(TRAY_APPKIT) + CGEventRef event = CGEventCreateKeyboardEvent(NULL, kVK_Escape, true); + CGEventPost(kCGHIDEventTap, event); + CFRelease(event); + CGEventRef event2 = CGEventCreateKeyboardEvent(NULL, kVK_Escape, false); + CGEventPost(kCGHIDEventTap, event2); + CFRelease(event2); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); +#endif + tray_exit(); + }); - // Toggle back + tray_show_menu(); + tray_loop(1); + capture_unchecked.join(); + + // Restore initial checked state testTray.menu[1].checked = 1; - tray_update(&testTray); } TEST_F(TrayTest, TestMultipleIconUpdates) { @@ -473,20 +512,12 @@ TEST_F(TrayTest, TestMultipleIconUpdates) { trayRunning = (initResult == 0); ASSERT_EQ(initResult, 0); - // Capture initial icon - WaitForTrayReady(); - EXPECT_TRUE(captureScreenshot("tray_icon_state1")); - // Update icon multiple times testTray.icon = TRAY_ICON2; tray_update(&testTray); - WaitForTrayReady(); - EXPECT_TRUE(captureScreenshot("tray_icon_state2")); testTray.icon = TRAY_ICON1; tray_update(&testTray); - WaitForTrayReady(); - EXPECT_TRUE(captureScreenshot("tray_icon_state3")); } TEST_F(TrayTest, TestCompleteMenuHierarchy) { @@ -505,9 +536,6 @@ TEST_F(TrayTest, TestCompleteMenuHierarchy) { ASSERT_NE(testTray.menu[4].submenu, nullptr); ASSERT_NE(testTray.menu[4].submenu[0].submenu, nullptr); ASSERT_NE(testTray.menu[4].submenu[1].submenu, nullptr); - - WaitForTrayReady(); - EXPECT_TRUE(captureScreenshot("tray_complete_menu_hierarchy")); } TEST_F(TrayTest, TestIconPathArray) { @@ -538,14 +566,10 @@ TEST_F(TrayTest, TestIconPathArray) { // Verify initial icon EXPECT_EQ(iconCacheTray->icon, TRAY_ICON1); - WaitForTrayReady(); - EXPECT_TRUE(captureScreenshot("tray_icon_cache_initial")); // Switch to cached icon iconCacheTray->icon = TRAY_ICON2; tray_update(iconCacheTray); - WaitForTrayReady(); - EXPECT_TRUE(captureScreenshot("tray_icon_cache_updated")); free(iconCacheTray); #else // On non-Windows platforms, just test basic icon switching @@ -554,13 +578,9 @@ TEST_F(TrayTest, TestIconPathArray) { ASSERT_EQ(initResult, 0); EXPECT_EQ(testTray.icon, TRAY_ICON1); - WaitForTrayReady(); - EXPECT_TRUE(captureScreenshot("tray_icon_cache_initial")); testTray.icon = TRAY_ICON2; tray_update(&testTray); - WaitForTrayReady(); - EXPECT_TRUE(captureScreenshot("tray_icon_cache_updated")); #endif } @@ -573,8 +593,6 @@ TEST_F(TrayTest, TestQuitCallback) { ASSERT_NE(testTray.menu[6].cb, nullptr); EXPECT_STREQ(testTray.menu[6].text, "Quit"); - WaitForTrayReady(); - EXPECT_TRUE(captureScreenshot("tray_before_quit")); // Note: Actually calling quit_cb would terminate the tray, // which is tested separately in TestTrayExit From 95ca118c37820030fb7ccf09402c57268f860f21 Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Sun, 22 Mar 2026 17:59:00 -0400 Subject: [PATCH 15/19] Refactor tray popup handling and update tests Introduce a dedicated _tray_popup helper on Linux to manage popup menus and their anchor window: track current_popup and menu_anchor_window, dismiss previous popups, create/destroy a tiny toplevel anchor, and use gtk_menu_popup_at_rect when available. Ensure cleanup of popups and anchor window in tray exit. Change Windows tray_show_menu to PostMessage to avoid synchronous SendMessage reentrancy. Minor whitespace/format adjustments in tray.h and tray_darwin.m. Update unit tests to remove fragile fixed sleeps and single-step tray_loop(1) calls: replace with polling while(tray_loop(0) == 0) and simplify screenshot/capture threads and platform-specific dismissal sequences to make tests more reliable. --- src/tray_linux.c | 94 ++++++++++++++++++++++++---------------- src/tray_windows.c | 3 +- tests/unit/test_tray.cpp | 49 +++++---------------- 3 files changed, 67 insertions(+), 79 deletions(-) diff --git a/src/tray_linux.c b/src/tray_linux.c index 2ec5a2a..84baf86 100644 --- a/src/tray_linux.c +++ b/src/tray_linux.c @@ -35,6 +35,8 @@ static AppIndicator *indicator = NULL; static int loop_result = 0; static NotifyNotification *currentNotification = NULL; static GtkMenu *current_menu = NULL; +static GtkMenu *current_popup = NULL; +static GtkWidget *menu_anchor_window = NULL; static void _tray_menu_cb(GtkMenuItem *item, gpointer data) { (void) item; @@ -171,52 +173,58 @@ void tray_update(struct tray *tray) { } } -void tray_show_menu(void) { - if (current_menu != NULL) { - // For AppIndicator, we need to show the menu manually since the indicator - // is managed by the system. In headless environments, we need to create - // a proper toplevel window for the menu to attach to. - - // Create an invisible toplevel window as an anchor - GtkWidget *anchor_window = gtk_window_new(GTK_WINDOW_TOPLEVEL); - if (anchor_window != NULL) { - gtk_window_set_type_hint(GTK_WINDOW(anchor_window), GDK_WINDOW_TYPE_HINT_POPUP_MENU); - gtk_window_set_decorated(GTK_WINDOW(anchor_window), FALSE); - gtk_window_set_skip_taskbar_hint(GTK_WINDOW(anchor_window), TRUE); - gtk_window_set_skip_pager_hint(GTK_WINDOW(anchor_window), TRUE); - gtk_window_move(GTK_WINDOW(anchor_window), 100, 100); - gtk_window_resize(GTK_WINDOW(anchor_window), 1, 1); - gtk_widget_show(anchor_window); - - // Give the window time to appear - while (gtk_events_pending()) { - gtk_main_iteration(); - } +static void _tray_popup(GtkMenu *menu) { + if (menu == NULL) { + return; + } + + // Dismiss any previously shown popup + if (current_popup != NULL) { + gtk_menu_popdown(current_popup); + current_popup = NULL; + } + if (menu_anchor_window != NULL) { + gtk_widget_destroy(menu_anchor_window); + menu_anchor_window = NULL; + } + + GtkWidget *anchor_window = gtk_window_new(GTK_WINDOW_TOPLEVEL); + if (anchor_window != NULL) { + gtk_window_set_type_hint(GTK_WINDOW(anchor_window), GDK_WINDOW_TYPE_HINT_POPUP_MENU); + gtk_window_set_decorated(GTK_WINDOW(anchor_window), FALSE); + gtk_window_set_skip_taskbar_hint(GTK_WINDOW(anchor_window), TRUE); + gtk_window_set_skip_pager_hint(GTK_WINDOW(anchor_window), TRUE); + gtk_window_move(GTK_WINDOW(anchor_window), 100, 100); + gtk_window_resize(GTK_WINDOW(anchor_window), 1, 1); + gtk_widget_show(anchor_window); + menu_anchor_window = anchor_window; + + while (gtk_events_pending()) { + gtk_main_iteration(); + } - if (gtk_check_version(3, 22, 0) == NULL) { - // GTK 3.22+ - use modern API with the anchor window - GdkWindow *gdk_window = gtk_widget_get_window(anchor_window); - if (gdk_window != NULL) { - GdkRectangle rect = {0, 0, 1, 1}; - gtk_menu_popup_at_rect(current_menu, gdk_window, &rect, GDK_GRAVITY_NORTH_WEST, GDK_GRAVITY_NORTH_WEST, NULL); - } else { - // Fallback - gtk_menu_popup(current_menu, NULL, NULL, NULL, NULL, 0, gtk_get_current_event_time()); - } + if (gtk_check_version(3, 22, 0) == NULL) { + GdkWindow *gdk_window = gtk_widget_get_window(anchor_window); + if (gdk_window != NULL) { + GdkRectangle rect = {0, 0, 1, 1}; + gtk_menu_popup_at_rect(menu, gdk_window, &rect, GDK_GRAVITY_NORTH_WEST, GDK_GRAVITY_NORTH_WEST, NULL); } else { - // Older GTK - use deprecated API - gtk_menu_popup(current_menu, NULL, NULL, NULL, NULL, 0, gtk_get_current_event_time()); + gtk_menu_popup(menu, NULL, NULL, NULL, NULL, 0, gtk_get_current_event_time()); } - - // Note: We don't destroy anchor_window here as the menu needs it to stay visible - // It will be cleaned up when the application exits } else { - // Fallback if window creation fails - gtk_menu_popup(current_menu, NULL, NULL, NULL, NULL, 0, gtk_get_current_event_time()); + gtk_menu_popup(menu, NULL, NULL, NULL, NULL, 0, gtk_get_current_event_time()); } + current_popup = menu; + } else { + gtk_menu_popup(menu, NULL, NULL, NULL, NULL, 0, gtk_get_current_event_time()); + current_popup = menu; } } +void tray_show_menu(void) { + _tray_popup(current_menu); +} + static gboolean tray_exit_internal(gpointer user_data) { (void) user_data; @@ -228,11 +236,21 @@ static gboolean tray_exit_internal(gpointer user_data) { currentNotification = NULL; } + if (current_popup != NULL) { + gtk_menu_popdown(current_popup); + current_popup = NULL; + } + if (current_menu != NULL) { g_object_unref(current_menu); current_menu = NULL; } + if (menu_anchor_window != NULL) { + gtk_widget_destroy(menu_anchor_window); + menu_anchor_window = NULL; + } + if (indicator != NULL) { // Make the indicator passive before unref to encourage a clean DBus unexport. app_indicator_set_status(indicator, APP_INDICATOR_STATUS_PASSIVE); diff --git a/src/tray_windows.c b/src/tray_windows.c index 1f66588..79c589f 100644 --- a/src/tray_windows.c +++ b/src/tray_windows.c @@ -316,8 +316,7 @@ void tray_update(struct tray *tray) { } void tray_show_menu(void) { - // Simulate a right-click on the tray icon to show the menu - SendMessage(hwnd, WM_TRAY_CALLBACK_MESSAGE, 0, WM_RBUTTONUP); + PostMessage(hwnd, WM_TRAY_CALLBACK_MESSAGE, 0, WM_RBUTTONUP); } void tray_exit(void) { diff --git a/tests/unit/test_tray.cpp b/tests/unit/test_tray.cpp index 6d21fc3..fd03232 100644 --- a/tests/unit/test_tray.cpp +++ b/tests/unit/test_tray.cpp @@ -279,29 +279,6 @@ TEST_F(TrayTest, TestSubmenuStructure) { EXPECT_STREQ(testTray.menu[4].submenu[0].text, "THIRD"); ASSERT_NE(testTray.menu[4].submenu[0].submenu, nullptr); EXPECT_STREQ(testTray.menu[4].submenu[0].submenu[0].text, "7"); - - // Show the menu open to capture nested menus visually - std::thread capture_thread([this]() { - std::this_thread::sleep_for(std::chrono::milliseconds(500)); - EXPECT_TRUE(captureScreenshot("tray_menu_with_submenu")); -#if defined(TRAY_WINAPI) - PostMessage(tray_get_hwnd(), WM_CANCELMODE, 0, 0); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); -#elif defined(TRAY_APPKIT) - CGEventRef event = CGEventCreateKeyboardEvent(NULL, kVK_Escape, true); - CGEventPost(kCGHIDEventTap, event); - CFRelease(event); - CGEventRef event2 = CGEventCreateKeyboardEvent(NULL, kVK_Escape, false); - CGEventPost(kCGHIDEventTap, event2); - CFRelease(event2); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); -#endif - tray_exit(); - }); - - tray_show_menu(); - tray_loop(1); - capture_thread.join(); } TEST_F(TrayTest, TestSubmenuCallback) { @@ -452,7 +429,6 @@ TEST_F(TrayTest, TestCheckboxStates) { // Show menu open with checkbox in checked state std::thread capture_checked([this]() { - std::this_thread::sleep_for(std::chrono::milliseconds(500)); EXPECT_TRUE(captureScreenshot("tray_menu_checkbox_checked")); #if defined(TRAY_WINAPI) PostMessage(tray_get_hwnd(), WM_CANCELMODE, 0, 0); @@ -470,7 +446,9 @@ TEST_F(TrayTest, TestCheckboxStates) { }); tray_show_menu(); - tray_loop(1); + while (tray_loop(0) == 0) { + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } capture_checked.join(); // Re-initialize tray with checkbox unchecked @@ -482,7 +460,6 @@ TEST_F(TrayTest, TestCheckboxStates) { // Show menu open with checkbox in unchecked state std::thread capture_unchecked([this]() { - std::this_thread::sleep_for(std::chrono::milliseconds(500)); EXPECT_TRUE(captureScreenshot("tray_menu_checkbox_unchecked")); #if defined(TRAY_WINAPI) PostMessage(tray_get_hwnd(), WM_CANCELMODE, 0, 0); @@ -500,7 +477,9 @@ TEST_F(TrayTest, TestCheckboxStates) { }); tray_show_menu(); - tray_loop(1); + while (tray_loop(0) == 0) { + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } capture_unchecked.join(); // Restore initial checked state @@ -593,7 +572,6 @@ TEST_F(TrayTest, TestQuitCallback) { ASSERT_NE(testTray.menu[6].cb, nullptr); EXPECT_STREQ(testTray.menu[6].text, "Quit"); - // Note: Actually calling quit_cb would terminate the tray, // which is tested separately in TestTrayExit } @@ -603,35 +581,28 @@ TEST_F(TrayTest, TestTrayShowMenu) { trayRunning = (initResult == 0); ASSERT_EQ(initResult, 0); - // Start a thread to capture screenshot and exit + // Screenshot shows the full menu open, including the SubMenu entry that leads to nested items std::thread capture_thread([this]() { - std::this_thread::sleep_for(std::chrono::milliseconds(500)); - WaitForTrayReady(); EXPECT_TRUE(captureScreenshot("tray_menu_shown")); #if defined(TRAY_WINAPI) - // Cancel the menu PostMessage(tray_get_hwnd(), WM_CANCELMODE, 0, 0); - // Wait for menu to close std::this_thread::sleep_for(std::chrono::milliseconds(100)); #elif defined(TRAY_APPKIT) - // Simulate ESC key to dismiss menu CGEventRef event = CGEventCreateKeyboardEvent(NULL, kVK_Escape, true); CGEventPost(kCGHIDEventTap, event); CFRelease(event); CGEventRef event2 = CGEventCreateKeyboardEvent(NULL, kVK_Escape, false); CGEventPost(kCGHIDEventTap, event2); CFRelease(event2); - // Wait for menu to close std::this_thread::sleep_for(std::chrono::milliseconds(100)); #endif - // Exit the tray tray_exit(); }); - // Show the menu programmatically tray_show_menu(); - - tray_loop(1); + while (tray_loop(0) == 0) { + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } capture_thread.join(); } From b3ebc6fea598fbc939324783272719ecfdf38339 Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Sun, 22 Mar 2026 18:51:55 -0400 Subject: [PATCH 16/19] style: sonar fixes --- src/tray_linux.c | 6 +- tests/conftest.cpp | 36 ++--- tests/screenshot_utils.cpp | 48 ++++--- tests/unit/test_tray.cpp | 286 +++++++++++++++++++------------------ 4 files changed, 182 insertions(+), 194 deletions(-) diff --git a/src/tray_linux.c b/src/tray_linux.c index 84baf86..3b9c678 100644 --- a/src/tray_linux.c +++ b/src/tray_linux.c @@ -9,6 +9,9 @@ #include #include +// local includes +#include "tray.h" + // lib includes #ifdef TRAY_AYATANA_APPINDICATOR #include @@ -24,9 +27,6 @@ // tray instances in the same desktop/session. static unsigned long tray_appindicator_seq = 0; -// local includes -#include "tray.h" - static bool async_update_pending = false; static pthread_cond_t async_update_cv = PTHREAD_COND_INITIALIZER; static pthread_mutex_t async_update_mutex = PTHREAD_MUTEX_INITIALIZER; diff --git a/tests/conftest.cpp b/tests/conftest.cpp index e5f04c4..6ae7c7b 100644 --- a/tests/conftest.cpp +++ b/tests/conftest.cpp @@ -33,13 +33,7 @@ class BaseTest: public ::testing::Test { // we can possibly use some internal googletest functions to capture stdout and stderr, but I have not tested this // https://stackoverflow.com/a/33186201/11214013 - BaseTest(): - sbuf {nullptr}, - pipe_stdout {nullptr}, - pipe_stderr {nullptr}, - screenshotsReady {false} { - // intentionally empty - } + BaseTest() = default; ~BaseTest() override = default; @@ -104,10 +98,10 @@ class BaseTest: public ::testing::Test { std::stringstream cout_buffer; // declare cout_buffer std::stringstream stdout_buffer; // declare stdout_buffer std::stringstream stderr_buffer; // declare stderr_buffer - std::streambuf *sbuf; - FILE *pipe_stdout; - FILE *pipe_stderr; - bool screenshotsReady; + std::streambuf *sbuf {nullptr}; + FILE *pipe_stdout {nullptr}; + FILE *pipe_stderr {nullptr}; + bool screenshotsReady {false}; void initializeScreenshotsOnce() { static std::once_flag screenshotInitFlag; @@ -147,13 +141,11 @@ class BaseTest: public ::testing::Test { if (screenshotsReady) { return true; } - std::string reason; - if (!screenshot::is_available(&reason)) { + if (std::string reason; !screenshot::is_available(&reason)) { screenshotUnavailableReason = reason; return false; } - auto root = screenshot::output_root(); - if (root.empty()) { + if (const auto root = screenshot::output_root(); root.empty()) { screenshotUnavailableReason = "Screenshot output directory not initialized"; return false; } @@ -187,10 +179,6 @@ class LinuxTest: public BaseTest { #endif BaseTest::SetUp(); } - - void TearDown() override { - BaseTest::TearDown(); - } }; class MacOSTest: public BaseTest { @@ -201,22 +189,14 @@ class MacOSTest: public BaseTest { #endif BaseTest::SetUp(); } - - void TearDown() override { - BaseTest::TearDown(); - } }; class WindowsTest: public BaseTest { protected: - void SetUp() override { + void SetUp() override { // NOSONAR(cpp:S1185) - contains platform skip logic, not a trivial override #ifndef _WIN32 GTEST_SKIP_("Skipping, this test is for Windows only."); #endif BaseTest::SetUp(); } - - void TearDown() override { - BaseTest::TearDown(); - } }; diff --git a/tests/screenshot_utils.cpp b/tests/screenshot_utils.cpp index 1243d38..8dddc27 100644 --- a/tests/screenshot_utils.cpp +++ b/tests/screenshot_utils.cpp @@ -22,8 +22,7 @@ #endif namespace { - std::filesystem::path g_outputRoot; - +#if defined(__linux__) || defined(__APPLE__) std::string quote_shell_path(const std::filesystem::path &path) { const std::string input = path.string(); std::string output; @@ -31,7 +30,7 @@ namespace { output.push_back('"'); for (const char ch : input) { if (ch == '"') { - output.append("\\\""); + output.append(R"(\")"); } else { output.push_back(ch); } @@ -39,15 +38,13 @@ namespace { output.push_back('"'); return output; } +#endif #ifdef _WIN32 - std::once_flag gdiplusInitFlag; - bool gdiplusReady = false; - ULONG_PTR gdiplusToken = 0; - std::once_flag dpiFlag; - bool dpiAware = false; - bool ensure_gdiplus() { + static std::once_flag gdiplusInitFlag; + static bool gdiplusReady = false; + static ULONG_PTR gdiplusToken = 0; std::call_once(gdiplusInitFlag, []() { Gdiplus::GdiplusStartupInput input; gdiplusReady = Gdiplus::GdiplusStartup(&gdiplusToken, &input, nullptr) == Gdiplus::Ok; @@ -56,9 +53,14 @@ namespace { } bool ensure_dpi_awareness() { + static std::once_flag dpiFlag; + static bool dpiAware = false; std::call_once(dpiFlag, []() { - auto setDPIAware = reinterpret_cast(GetProcAddress(GetModuleHandleA("user32.dll"), "SetProcessDPIAware")); - dpiAware = setDPIAware == nullptr || setDPIAware() == TRUE; + 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; }); return dpiAware; } @@ -70,7 +72,7 @@ namespace { return false; } std::vector buffer(size); - auto info = reinterpret_cast(buffer.data()); + auto *info = static_cast(static_cast(buffer.data())); if (Gdiplus::GetImageEncoders(num, size, info) != Gdiplus::Ok) { return false; } @@ -82,20 +84,24 @@ namespace { } return false; } - #endif } // namespace namespace screenshot { + inline std::filesystem::path &output_root_ref() { + static std::filesystem::path g_outputRoot; + return g_outputRoot; + } + void initialize(const std::filesystem::path &rootDir) { - g_outputRoot = rootDir / "screenshots"; + output_root_ref() = rootDir / "screenshots"; std::error_code ec; - std::filesystem::create_directories(g_outputRoot, ec); + std::filesystem::create_directories(output_root_ref(), ec); } std::filesystem::path output_root() { - return g_outputRoot; + return output_root_ref(); } #ifdef __APPLE__ @@ -135,8 +141,7 @@ namespace screenshot { int width = GetSystemMetrics(SM_CXVIRTUALSCREEN); int height = GetSystemMetrics(SM_CYVIRTUALSCREEN); - HWND desktop = GetDesktopWindow(); - if ((width <= 0 || height <= 0) && desktop != nullptr) { + if (HWND desktop = GetDesktopWindow(); (width <= 0 || height <= 0) && desktop != nullptr) { RECT rect {}; if (GetWindowRect(desktop, &rect)) { left = rect.left; @@ -187,8 +192,7 @@ namespace screenshot { std::cerr << "PNG encoder CLSID not found" << std::endl; return false; } - const std::wstring widePath = file.wstring(); - if (bitmap.Save(widePath.c_str(), &pngClsid, nullptr) != Gdiplus::Ok) { + if (bitmap.Save(file.wstring().c_str(), &pngClsid, nullptr) != Gdiplus::Ok) { std::cerr << "GDI+ failed to write " << file << std::endl; return false; } @@ -227,10 +231,10 @@ namespace screenshot { // Add a delay to allow UI elements to render before capturing std::this_thread::sleep_for(std::chrono::milliseconds(500)); - if (g_outputRoot.empty()) { + if (output_root_ref().empty()) { return false; } - auto file = g_outputRoot / (name + ".png"); + auto file = output_root_ref() / (name + ".png"); #ifdef __APPLE__ return capture_macos(file, options); diff --git a/tests/unit/test_tray.cpp b/tests/unit/test_tray.cpp index fd03232..4bb6f80 100644 --- a/tests/unit/test_tray.cpp +++ b/tests/unit/test_tray.cpp @@ -2,8 +2,11 @@ #include "tests/conftest.cpp" // standard includes +#include #include +#include #include +#include #if defined(_WIN32) || defined(_WIN64) #include @@ -24,37 +27,78 @@ #include "tests/screenshot_utils.h" #if TRAY_APPINDICATOR - #define TRAY_ICON1 "mail-message-new" - #define TRAY_ICON2 "mail-message-new" +constexpr const char *TRAY_ICON1 = "mail-message-new"; +constexpr const char *TRAY_ICON2 = "mail-message-new"; #elif TRAY_APPKIT - #define TRAY_ICON1 "icon.png" - #define TRAY_ICON2 "icon.png" +constexpr const char *TRAY_ICON1 = "icon.png"; +constexpr const char *TRAY_ICON2 = "icon.png"; #elif TRAY_WINAPI - #define TRAY_ICON1 "icon.ico" - #define TRAY_ICON2 "icon.ico" +constexpr const char *TRAY_ICON1 = "icon.ico"; +constexpr const char *TRAY_ICON2 = "icon.ico"; #endif +// File-scope tray data shared across all TrayTest instances +namespace { + // NOSONAR(cpp:S5945, cpp:S5421) - C-style arrays with null sentinel are required by the tray C API; + // mutable (non-const) because callbacks are assigned at runtime in SetUp() + struct tray_menu g_submenu7_8[] = { + {.text = "7", .cb = nullptr}, + {.text = "-"}, + {.text = "8", .cb = nullptr}, + {.text = nullptr} + }; + struct tray_menu g_submenu5_6[] = { + {.text = "5", .cb = nullptr}, + {.text = "6", .cb = nullptr}, + {.text = nullptr} + }; + struct tray_menu g_submenu_second[] = { + {.text = "THIRD", .submenu = g_submenu7_8}, + {.text = "FOUR", .submenu = g_submenu5_6}, + {.text = nullptr} + }; + struct tray_menu g_submenu[] = { + {.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 = { + .icon = TRAY_ICON1, + .tooltip = "TestTray", + .menu = g_submenu + }; +} // namespace + class TrayTest: public BaseTest { + void ShutdownTray() { + if (!trayRunning) { + return; + } + tray_exit(); + tray_loop(0); + trayRunning = false; + } + protected: - static struct tray testTray; - bool trayRunning; - std::string iconPath1; - std::string iconPath2; - - // Static arrays for submenus - static struct tray_menu submenu7_8[]; - static struct tray_menu submenu5_6[]; - static struct tray_menu submenu_second[]; - static struct tray_menu submenu[]; - - // Non-static member functions - static void hello_cb(struct tray_menu *item) { + bool trayRunning {false}; // NOSONAR(cpp:S3656) - protected access required by gtest TEST_F subclass pattern + struct tray &testTray = g_testTray; + struct tray_menu *submenu = g_submenu; + struct tray_menu *submenu7_8 = g_submenu7_8; + struct tray_menu *submenu5_6 = g_submenu5_6; + struct tray_menu *submenu_second = g_submenu_second; + + static void hello_cb([[maybe_unused]] struct tray_menu *item) { // Mock implementation } static void toggle_cb(struct tray_menu *item) { - item->checked = !item->checked; - tray_update(&testTray); + g_testTray.menu[1].checked = !g_testTray.menu[1].checked; + tray_update(&g_testTray); } static void quit_cb(struct tray_menu *item) { @@ -63,12 +107,21 @@ class TrayTest: public BaseTest { static void submenu_cb(struct tray_menu *item) { // Mock implementation - tray_update(&testTray); + tray_update(&g_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; @@ -79,24 +132,17 @@ class TrayTest: public BaseTest { #if defined(TRAY_WINAPI) || defined(TRAY_APPKIT) // Ensure icon files exist in test binary directory - // Look for icons in project root or cmake build directory std::filesystem::path projectRoot = testBinaryDir.parent_path(); std::filesystem::path iconSource; - // Try icons directory first if (std::filesystem::exists(projectRoot / "icons" / TRAY_ICON1)) { iconSource = projectRoot / "icons" / TRAY_ICON1; - } - // Try project root - else if (std::filesystem::exists(projectRoot / TRAY_ICON1)) { + } else if (std::filesystem::exists(projectRoot / TRAY_ICON1)) { iconSource = projectRoot / TRAY_ICON1; - } - // Try current directory - else if (std::filesystem::exists(std::filesystem::path(TRAY_ICON1))) { + } else if (std::filesystem::exists(std::filesystem::path(TRAY_ICON1))) { iconSource = std::filesystem::path(TRAY_ICON1); } - // Copy icon to test binary directory if not already there if (!iconSource.empty()) { std::filesystem::path iconDest = testBinaryDir / TRAY_ICON1; if (!std::filesystem::exists(iconDest)) { @@ -112,8 +158,8 @@ class TrayTest: public BaseTest { trayRunning = false; testTray.icon = TRAY_ICON1; testTray.tooltip = "TestTray"; - testTray.menu = submenu; - submenu[1].checked = 1; // Reset checkbox state to initial value + testTray.menu = g_submenu; + g_submenu[1].checked = 1; } void TearDown() override { @@ -121,75 +167,28 @@ class TrayTest: public BaseTest { BaseTest::TearDown(); } - void ShutdownTray() { - if (!trayRunning) { - return; - } - tray_exit(); - tray_loop(0); - trayRunning = false; - } - - // Process pending events to allow tray icon to appear - // Call this ONLY before screenshots to ensure the icon is visible + // Process pending events to allow tray icon to appear. + // Call this ONLY before screenshots to ensure the icon is visible. void WaitForTrayReady() { #if defined(TRAY_APPINDICATOR) - // On Linux: process GTK events to allow AppIndicator D-Bus registration for (int i = 0; i < 100; i++) { - tray_loop(0); // Non-blocking - process pending events + tray_loop(0); std::this_thread::sleep_for(std::chrono::milliseconds(5)); } #elif defined(TRAY_APPKIT) - // On macOS: process events if on main thread, otherwise just sleep - // NSApp event loop must only be called from main thread static std::thread::id main_thread_id = std::this_thread::get_id(); if (std::this_thread::get_id() == main_thread_id) { - // Main thread - safe to call tray_loop for (int i = 0; i < 100; i++) { - tray_loop(0); // Non-blocking - process pending events + tray_loop(0); std::this_thread::sleep_for(std::chrono::milliseconds(5)); } } else { - // Background thread - just wait longer std::this_thread::sleep_for(std::chrono::milliseconds(1000)); } #endif } }; -// Define the static arrays -struct tray_menu TrayTest::submenu7_8[] = { - {.text = "7", .cb = submenu_cb}, - {.text = "-"}, - {.text = "8", .cb = submenu_cb}, - {.text = nullptr} -}; -struct tray_menu TrayTest::submenu5_6[] = { - {.text = "5", .cb = submenu_cb}, - {.text = "6", .cb = submenu_cb}, - {.text = nullptr} -}; -struct tray_menu TrayTest::submenu_second[] = { - {.text = "THIRD", .submenu = submenu7_8}, - {.text = "FOUR", .submenu = submenu5_6}, - {.text = nullptr} -}; -struct tray_menu TrayTest::submenu[] = { - {.text = "Hello", .cb = hello_cb}, - {.text = "Checked", .checked = 1, .checkbox = 1, .cb = toggle_cb}, - {.text = "Disabled", .disabled = 1}, - {.text = "-"}, - {.text = "SubMenu", .submenu = submenu_second}, - {.text = "-"}, - {.text = "Quit", .cb = quit_cb}, - {.text = nullptr} -}; -struct tray TrayTest::testTray = { - .icon = TRAY_ICON1, - .tooltip = "TestTray", - .menu = submenu -}; - TEST_F(TrayTest, TestTrayInit) { int result = tray_init(&testTray); trayRunning = (result == 0); @@ -298,8 +297,8 @@ TEST_F(TrayTest, TestNotificationDisplay) { #if defined(_WIN32) QUERY_USER_NOTIFICATION_STATE notification_state; - HRESULT ns = SHQueryUserNotificationState(¬ification_state); - if (ns != S_OK || notification_state != QUNS_ACCEPTS_NOTIFICATIONS) { + if (HRESULT ns = SHQueryUserNotificationState(¬ification_state); + ns != S_OK || notification_state != QUNS_ACCEPTS_NOTIFICATIONS) { GTEST_SKIP() << "Notifications not accepted in this environment. SHQueryUserNotificationState result: " << ns << ", state: " << notification_state; } #endif @@ -332,8 +331,8 @@ TEST_F(TrayTest, TestNotificationCallback) { #if defined(_WIN32) QUERY_USER_NOTIFICATION_STATE notification_state; - HRESULT ns = SHQueryUserNotificationState(¬ification_state); - if (ns != S_OK || notification_state != QUNS_ACCEPTS_NOTIFICATIONS) { + if (HRESULT ns = SHQueryUserNotificationState(¬ification_state); + ns != S_OK || notification_state != QUNS_ACCEPTS_NOTIFICATIONS) { GTEST_SKIP() << "Notifications not accepted in this environment. SHQueryUserNotificationState result: " << ns << ", state: " << notification_state; } #endif @@ -389,18 +388,16 @@ TEST_F(TrayTest, TestMenuItemContext) { static int contextValue = 42; static bool contextCallbackInvoked = false; - auto context_callback = [](struct tray_menu *item) { + auto context_callback = [](struct tray_menu *item) { // NOSONAR(cpp:S995) - must match tray_menu.cb signature void(*)(struct tray_menu*) if (item->context != nullptr) { - int *value = static_cast(item->context); + const auto *value = static_cast(item->context); contextCallbackInvoked = (*value == 42); } }; // Create menu with context - struct tray_menu context_menu[] = { - {.text = "Context Item", .cb = context_callback, .context = &contextValue}, - {.text = nullptr} - }; + std::array context_menu_arr = {{{.text = "Context Item", .cb = context_callback, .context = &contextValue}, {.text = nullptr}}}; + struct tray_menu *context_menu = context_menu_arr.data(); testTray.menu = context_menu; @@ -428,28 +425,30 @@ TEST_F(TrayTest, TestCheckboxStates) { EXPECT_EQ(testTray.menu[1].checked, 1); // Show menu open with checkbox in checked state - std::thread capture_checked([this]() { - EXPECT_TRUE(captureScreenshot("tray_menu_checkbox_checked")); + { + std::thread capture_checked([this]() { // NOSONAR(cpp:S6168) - std::jthread unavailable on AppleClang 17 (libc++ < 18) + EXPECT_TRUE(captureScreenshot("tray_menu_checkbox_checked")); #if defined(TRAY_WINAPI) - PostMessage(tray_get_hwnd(), WM_CANCELMODE, 0, 0); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); + PostMessage(tray_get_hwnd(), WM_CANCELMODE, 0, 0); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); #elif defined(TRAY_APPKIT) - CGEventRef event = CGEventCreateKeyboardEvent(NULL, kVK_Escape, true); - CGEventPost(kCGHIDEventTap, event); - CFRelease(event); - CGEventRef event2 = CGEventCreateKeyboardEvent(NULL, kVK_Escape, false); - CGEventPost(kCGHIDEventTap, event2); - CFRelease(event2); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); + CGEventRef event = CGEventCreateKeyboardEvent(NULL, kVK_Escape, true); + CGEventPost(kCGHIDEventTap, event); + CFRelease(event); + CGEventRef event2 = CGEventCreateKeyboardEvent(NULL, kVK_Escape, false); + CGEventPost(kCGHIDEventTap, event2); + CFRelease(event2); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); #endif - tray_exit(); - }); + tray_exit(); + }); - tray_show_menu(); - while (tray_loop(0) == 0) { - std::this_thread::sleep_for(std::chrono::milliseconds(10)); + tray_show_menu(); + while (tray_loop(0) == 0) { + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } + capture_checked.join(); } - capture_checked.join(); // Re-initialize tray with checkbox unchecked trayRunning = false; @@ -459,28 +458,30 @@ TEST_F(TrayTest, TestCheckboxStates) { ASSERT_EQ(initResult, 0); // Show menu open with checkbox in unchecked state - std::thread capture_unchecked([this]() { - EXPECT_TRUE(captureScreenshot("tray_menu_checkbox_unchecked")); + { + std::thread capture_unchecked([this]() { // NOSONAR(cpp:S6168) - std::jthread unavailable on AppleClang 17 (libc++ < 18) + EXPECT_TRUE(captureScreenshot("tray_menu_checkbox_unchecked")); #if defined(TRAY_WINAPI) - PostMessage(tray_get_hwnd(), WM_CANCELMODE, 0, 0); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); + PostMessage(tray_get_hwnd(), WM_CANCELMODE, 0, 0); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); #elif defined(TRAY_APPKIT) - CGEventRef event = CGEventCreateKeyboardEvent(NULL, kVK_Escape, true); - CGEventPost(kCGHIDEventTap, event); - CFRelease(event); - CGEventRef event2 = CGEventCreateKeyboardEvent(NULL, kVK_Escape, false); - CGEventPost(kCGHIDEventTap, event2); - CFRelease(event2); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); + CGEventRef event = CGEventCreateKeyboardEvent(NULL, kVK_Escape, true); + CGEventPost(kCGHIDEventTap, event); + CFRelease(event); + CGEventRef event2 = CGEventCreateKeyboardEvent(NULL, kVK_Escape, false); + CGEventPost(kCGHIDEventTap, event2); + CFRelease(event2); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); #endif - tray_exit(); - }); + tray_exit(); + }); - tray_show_menu(); - while (tray_loop(0) == 0) { - std::this_thread::sleep_for(std::chrono::milliseconds(10)); + tray_show_menu(); + while (tray_loop(0) == 0) { + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } + capture_unchecked.join(); } - capture_unchecked.join(); // Restore initial checked state testTray.menu[1].checked = 1; @@ -506,7 +507,7 @@ TEST_F(TrayTest, TestCompleteMenuHierarchy) { // Verify complete menu structure int menuCount = 0; - for (struct tray_menu *m = testTray.menu; m->text != nullptr; m++) { + for (const struct tray_menu *m = testTray.menu; m->text != nullptr; m++) { menuCount++; } EXPECT_EQ(menuCount, 7); // Hello, Checked, Disabled, Sep, SubMenu, Sep, Quit @@ -520,14 +521,13 @@ TEST_F(TrayTest, TestCompleteMenuHierarchy) { TEST_F(TrayTest, TestIconPathArray) { #if defined(TRAY_WINAPI) // Test icon path array caching (Windows-specific feature) - // Allocate memory for tray struct with flexible array member + // The tray struct has a flexible array member, so we allocate a raw buffer + // and use memcpy to initialize const fields before the object is used. const size_t icon_count = 2; - struct tray *iconCacheTray = (struct tray *) malloc( - sizeof(struct tray) + icon_count * sizeof(const char *) - ); - ASSERT_NE(iconCacheTray, nullptr); + const size_t buf_size = sizeof(struct tray) + icon_count * sizeof(const char *); + std::vector buf(buf_size, std::byte {0}); + auto *iconCacheTray = reinterpret_cast(buf.data()); // NOSONAR(cpp:S3630) - reinterpret_cast required to overlay struct onto raw buffer for flexible array member - // Initialize the tray structure iconCacheTray->icon = TRAY_ICON1; iconCacheTray->tooltip = "Icon Cache Test"; iconCacheTray->notification_icon = nullptr; @@ -535,9 +535,14 @@ TEST_F(TrayTest, TestIconPathArray) { iconCacheTray->notification_title = nullptr; iconCacheTray->notification_cb = nullptr; iconCacheTray->menu = submenu; - *const_cast(&iconCacheTray->iconPathCount) = icon_count; - *const_cast(&iconCacheTray->allIconPaths[0]) = TRAY_ICON1; - *const_cast(&iconCacheTray->allIconPaths[1]) = TRAY_ICON2; + + // Write const fields via memcpy — const_cast is required to initialize const members in a C struct flexible array allocation + auto count_val = static_cast(icon_count); + std::memcpy(const_cast(&iconCacheTray->iconPathCount), &count_val, sizeof(count_val)); // NOSONAR(cpp:S859) - required to initialize const member in C struct allocated via raw buffer + const char *icon1 = TRAY_ICON1; + const char *icon2 = TRAY_ICON2; + std::memcpy(const_cast(&iconCacheTray->allIconPaths[0]), &icon1, sizeof(icon1)); // NOSONAR(cpp:S859) - required to initialize const member in C struct allocated via raw buffer + std::memcpy(const_cast(&iconCacheTray->allIconPaths[1]), &icon2, sizeof(icon2)); // NOSONAR(cpp:S859) - required to initialize const member in C struct allocated via raw buffer int initResult = tray_init(iconCacheTray); trayRunning = (initResult == 0); @@ -549,7 +554,7 @@ TEST_F(TrayTest, TestIconPathArray) { // Switch to cached icon iconCacheTray->icon = TRAY_ICON2; tray_update(iconCacheTray); - free(iconCacheTray); + // buf goes out of scope, no manual free needed #else // On non-Windows platforms, just test basic icon switching int initResult = tray_init(&testTray); @@ -582,7 +587,7 @@ TEST_F(TrayTest, TestTrayShowMenu) { ASSERT_EQ(initResult, 0); // Screenshot shows the full menu open, including the SubMenu entry that leads to nested items - std::thread capture_thread([this]() { + std::thread capture_thread([this]() { // NOSONAR(cpp:S6168) - std::jthread unavailable on AppleClang 17 (libc++ < 18) EXPECT_TRUE(captureScreenshot("tray_menu_shown")); #if defined(TRAY_WINAPI) PostMessage(tray_get_hwnd(), WM_CANCELMODE, 0, 0); @@ -603,7 +608,6 @@ TEST_F(TrayTest, TestTrayShowMenu) { while (tray_loop(0) == 0) { std::this_thread::sleep_for(std::chrono::milliseconds(10)); } - capture_thread.join(); } From 41f5ed44163a4b56b866dfba4918bdc1ce5afb77 Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Sun, 22 Mar 2026 19:46:43 -0400 Subject: [PATCH 17/19] style: more sonar fixes --- tests/screenshot_utils.cpp | 2 +- tests/unit/test_tray.cpp | 136 +++++++++++++------------------------ 2 files changed, 50 insertions(+), 88 deletions(-) diff --git a/tests/screenshot_utils.cpp b/tests/screenshot_utils.cpp index 8dddc27..859e831 100644 --- a/tests/screenshot_utils.cpp +++ b/tests/screenshot_utils.cpp @@ -90,7 +90,7 @@ namespace { namespace screenshot { inline std::filesystem::path &output_root_ref() { - static std::filesystem::path g_outputRoot; + static std::filesystem::path g_outputRoot; // NOSONAR(cpp:S6018) - function-local static is intentional for lazy, TU-local initialization return g_outputRoot; } diff --git a/tests/unit/test_tray.cpp b/tests/unit/test_tray.cpp index 4bb6f80..e6b082f 100644 --- a/tests/unit/test_tray.cpp +++ b/tests/unit/test_tray.cpp @@ -39,25 +39,23 @@ constexpr const char *TRAY_ICON2 = "icon.ico"; // File-scope tray data shared across all TrayTest instances namespace { - // NOSONAR(cpp:S5945, cpp:S5421) - C-style arrays with null sentinel are required by the tray C API; - // mutable (non-const) because callbacks are assigned at runtime in SetUp() - struct tray_menu g_submenu7_8[] = { + 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[] = { + 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[] = { + 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[] = { + 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}, @@ -67,14 +65,15 @@ namespace { {.text = "Quit", .cb = nullptr}, {.text = nullptr} }; - struct tray g_testTray = { + struct tray g_testTray = { // NOSONAR(cpp:S5421) - mutable global required for shared tray state across TEST_F instances .icon = TRAY_ICON1, .tooltip = "TestTray", .menu = g_submenu }; } // namespace -class TrayTest: public BaseTest { +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; @@ -84,28 +83,58 @@ class TrayTest: public BaseTest { trayRunning = false; } -protected: + // Dismisses the open menu and exits the tray event loop from a background thread. + void closeMenuAndExit() { +#if defined(TRAY_WINAPI) + PostMessage(tray_get_hwnd(), WM_CANCELMODE, 0, 0); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); +#elif defined(TRAY_APPKIT) + CGEventRef event = CGEventCreateKeyboardEvent(NULL, kVK_Escape, true); + CGEventPost(kCGHIDEventTap, event); + CFRelease(event); + CGEventRef event2 = CGEventCreateKeyboardEvent(NULL, kVK_Escape, false); + CGEventPost(kCGHIDEventTap, event2); + CFRelease(event2); + std::this_thread::sleep_for(std::chrono::milliseconds(100)); +#endif + tray_exit(); + } + + // Capture a screenshot while the tray menu is open, then dismiss and exit. + void captureMenuStateAndExit(const char *screenshotName) { + std::thread capture_thread([this, screenshotName]() { // NOSONAR(cpp:S6168) - std::jthread is unavailable on AppleClang 17/libc++ used in CI + EXPECT_TRUE(captureScreenshot(screenshotName)); + closeMenuAndExit(); + }); + + tray_show_menu(); + while (tray_loop(0) == 0) { + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } + capture_thread.join(); + } + bool trayRunning {false}; // NOSONAR(cpp:S3656) - protected access required by gtest TEST_F subclass pattern - struct tray &testTray = g_testTray; - struct tray_menu *submenu = g_submenu; - struct tray_menu *submenu7_8 = g_submenu7_8; - struct tray_menu *submenu5_6 = g_submenu5_6; - struct tray_menu *submenu_second = g_submenu_second; + 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) { // Mock implementation } - static void toggle_cb(struct tray_menu *item) { + 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 quit_cb(struct tray_menu *item) { + static void quit_cb([[maybe_unused]] struct tray_menu *item) { // NOSONAR(cpp:S1172) - unused param required by tray_menu.cb function pointer type tray_exit(); } - static void submenu_cb(struct tray_menu *item) { + static void submenu_cb([[maybe_unused]] struct tray_menu *item) { // NOSONAR(cpp:S1172) - unused param required by tray_menu.cb function pointer type // Mock implementation tray_update(&g_testTray); } @@ -425,30 +454,7 @@ TEST_F(TrayTest, TestCheckboxStates) { EXPECT_EQ(testTray.menu[1].checked, 1); // Show menu open with checkbox in checked state - { - std::thread capture_checked([this]() { // NOSONAR(cpp:S6168) - std::jthread unavailable on AppleClang 17 (libc++ < 18) - EXPECT_TRUE(captureScreenshot("tray_menu_checkbox_checked")); -#if defined(TRAY_WINAPI) - PostMessage(tray_get_hwnd(), WM_CANCELMODE, 0, 0); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); -#elif defined(TRAY_APPKIT) - CGEventRef event = CGEventCreateKeyboardEvent(NULL, kVK_Escape, true); - CGEventPost(kCGHIDEventTap, event); - CFRelease(event); - CGEventRef event2 = CGEventCreateKeyboardEvent(NULL, kVK_Escape, false); - CGEventPost(kCGHIDEventTap, event2); - CFRelease(event2); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); -#endif - tray_exit(); - }); - - tray_show_menu(); - while (tray_loop(0) == 0) { - std::this_thread::sleep_for(std::chrono::milliseconds(10)); - } - capture_checked.join(); - } + captureMenuStateAndExit("tray_menu_checkbox_checked"); // NOSONAR(cpp:S6168) - helper uses std::thread for AppleClang 17 compatibility // Re-initialize tray with checkbox unchecked trayRunning = false; @@ -458,30 +464,7 @@ TEST_F(TrayTest, TestCheckboxStates) { ASSERT_EQ(initResult, 0); // Show menu open with checkbox in unchecked state - { - std::thread capture_unchecked([this]() { // NOSONAR(cpp:S6168) - std::jthread unavailable on AppleClang 17 (libc++ < 18) - EXPECT_TRUE(captureScreenshot("tray_menu_checkbox_unchecked")); -#if defined(TRAY_WINAPI) - PostMessage(tray_get_hwnd(), WM_CANCELMODE, 0, 0); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); -#elif defined(TRAY_APPKIT) - CGEventRef event = CGEventCreateKeyboardEvent(NULL, kVK_Escape, true); - CGEventPost(kCGHIDEventTap, event); - CFRelease(event); - CGEventRef event2 = CGEventCreateKeyboardEvent(NULL, kVK_Escape, false); - CGEventPost(kCGHIDEventTap, event2); - CFRelease(event2); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); -#endif - tray_exit(); - }); - - tray_show_menu(); - while (tray_loop(0) == 0) { - std::this_thread::sleep_for(std::chrono::milliseconds(10)); - } - capture_unchecked.join(); - } + captureMenuStateAndExit("tray_menu_checkbox_unchecked"); // NOSONAR(cpp:S6168) - helper uses std::thread for AppleClang 17 compatibility // Restore initial checked state testTray.menu[1].checked = 1; @@ -587,28 +570,7 @@ TEST_F(TrayTest, TestTrayShowMenu) { ASSERT_EQ(initResult, 0); // Screenshot shows the full menu open, including the SubMenu entry that leads to nested items - std::thread capture_thread([this]() { // NOSONAR(cpp:S6168) - std::jthread unavailable on AppleClang 17 (libc++ < 18) - EXPECT_TRUE(captureScreenshot("tray_menu_shown")); -#if defined(TRAY_WINAPI) - PostMessage(tray_get_hwnd(), WM_CANCELMODE, 0, 0); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); -#elif defined(TRAY_APPKIT) - CGEventRef event = CGEventCreateKeyboardEvent(NULL, kVK_Escape, true); - CGEventPost(kCGHIDEventTap, event); - CFRelease(event); - CGEventRef event2 = CGEventCreateKeyboardEvent(NULL, kVK_Escape, false); - CGEventPost(kCGHIDEventTap, event2); - CFRelease(event2); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); -#endif - tray_exit(); - }); - - tray_show_menu(); - while (tray_loop(0) == 0) { - std::this_thread::sleep_for(std::chrono::milliseconds(10)); - } - capture_thread.join(); + captureMenuStateAndExit("tray_menu_shown"); // NOSONAR(cpp:S6168) - helper uses std::thread for AppleClang 17 compatibility } TEST_F(TrayTest, TestTrayExit) { From a517165f0612fd015d0cb89fa8ec30a42351677c Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Sun, 22 Mar 2026 20:04:35 -0400 Subject: [PATCH 18/19] Simplify macOS TCC configuration in CI Refactor the macOS screen-recording permission step in the CI workflow: enable strict shell flags, replace the previous dynamic find/loop logic with helper functions (configure_system_tccdb/configure_user_tccdb) and fixed arrays of SQL insert values, and insert entries directly into the system and user TCC databases. Removes verbose logging, Sonoma-specific branching, and dynamic path discovery in favor of a simpler, more deterministic insertion approach and adds a final confirmation echo. Co-Authored-By: Thomas Van Laere <4990509+thomvanl@users.noreply.github.com> --- .github/workflows/ci.yml | 82 +++++++++++----------------------------- 1 file changed, 23 insertions(+), 59 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b7c656a..781f6bf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -79,74 +79,38 @@ jobs: - name: Fix macOS screen recording permissions if: runner.os == 'macOS' run: | - # Grant screen recording permissions to prevent popup dialogs in CI - # This modifies the TCC (Transparency, Consent, and Control) database + set -euo pipefail - # https://apple.stackexchange.com/questions/362865/macos-list-apps-authorized-for-full-disk-access - # https://github.com/actions/runner-images/issues/9529 - # https://github.com/actions/runner-images/pull/9530 - - # Get macOS version - os_version=$(sw_vers -productVersion | cut -d '.' -f 1) - echo "macOS version: $os_version" - - # function to execute sql query for each value - function execute_sql_query { - local value=$1 - local dbPath=$2 - echo "Executing SQL query for value: $value" - sudo sqlite3 "$dbPath" "INSERT OR IGNORE INTO access VALUES($value);" + configure_system_tccdb() { + local values=$1 + local dbPath="/Library/Application Support/com.apple.TCC/TCC.db" + local sqlQuery="INSERT OR IGNORE INTO access VALUES($values);" + sudo sqlite3 "$dbPath" "$sqlQuery" } - # Find all provisioner paths and store them in an array - declare -a app_paths=() - while IFS= read -r line; do - app_paths+=("$line") - done < <(sudo find /opt /usr -name bash 2>/dev/null) - echo "Provisioner paths: ${app_paths[@]}" - - # Create an empty array - declare -a values=() + configure_user_tccdb() { + local values=$1 + local dbPath="$HOME/Library/Application Support/com.apple.TCC/TCC.db" + local sqlQuery="INSERT OR IGNORE INTO access VALUES($values);" + sqlite3 "$dbPath" "$sqlQuery" + } - # Loop through the provisioner paths and add them to the values array - for p_path in "${app_paths[@]}"; do - # Adjust the service name and other parameters as needed - values+=("'kTCCServiceAccessibility','${p_path}',1,2,4,1,NULL,NULL,0,'UNUSED',NULL,NULL,1592919552") - values+=("'kTCCServiceScreenCapture','${p_path}',1,2,4,1,NULL,NULL,0,'UNUSED',NULL,0,1687786159") + systemValuesArray=( + "'kTCCServiceScreenCapture','/bin/bash',1,2,0,1,NULL,NULL,NULL,'UNUSED',NULL,0,1599831148" + ) + for values in "${systemValuesArray[@]}"; do + configure_system_tccdb "$values,NULL,NULL,'UNUSED',${values##*,}" done - echo "Values: ${values[@]}" - # Adjust for Sonoma (macOS 14+) which has extra columns - if [[ "$os_version" -ge 14 ]]; then - echo "Adjusting for macOS Sonoma or later (extra TCC columns)" - # TCC access table in Sonoma has extra 4 columns: pid, pid_version, boot_uuid, last_reminded - for i in "${!values[@]}"; do - values[$i]="${values[$i]},NULL,NULL,'UNUSED',${values[$i]##*,}" - done - fi - - # System and user TCC databases - dbPaths=( - "/Library/Application Support/com.apple.TCC/TCC.db" - "$HOME/Library/Application Support/com.apple.TCC/TCC.db" + userValuesArray=( + "'kTCCServiceScreenCapture','/bin/bash',1,2,0,1,NULL,NULL,NULL,'UNUSED',NULL,0,1583997993" ) - - # Execute SQL queries - for value in "${values[@]}"; do - for dbPath in "${dbPaths[@]}"; do - echo "Column names for $dbPath" - echo "-------------------" - sudo sqlite3 "$dbPath" "PRAGMA table_info(access);" - echo "Current permissions for $dbPath" - echo "-------------------" - sudo sqlite3 "$dbPath" "SELECT * FROM access WHERE service='kTCCServiceScreenCapture';" - execute_sql_query "$value" "$dbPath" - echo "Updated permissions for $dbPath" - echo "-------------------" - sudo sqlite3 "$dbPath" "SELECT * FROM access WHERE service='kTCCServiceScreenCapture';" - done + for values in "${userValuesArray[@]}"; do + configure_user_tccdb "$values,NULL,NULL,'UNUSED',${values##*,}" done + echo "macOS TCC permissions configured." + - name: Setup Dependencies Windows if: runner.os == 'Windows' uses: msys2/setup-msys2@4f806de0a5a7294ffabaff804b38a9b435a73bda # v2.30.0 From 7b11e9590a5122a8eccbaa4a88c63eaefa215030 Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Sun, 22 Mar 2026 22:07:50 -0400 Subject: [PATCH 19/19] Enhance publish-screenshots workflow Pin artifact actions in CI and extend the screenshots publishing workflow. ci.yml: update upload-artifact usage to the v7 SHA. publish-screenshots.yml: add actions read permission, pin download-artifact and checkout actions, change artifact download to include repo/token and save to run-screenshots, and prepare per-matrix directories. Add steps to detect workflow context (PR vs master push), sync prepared screenshots into a screenshots branch (baseline or pull-requests/PR-) using rsync, generate a PR comparison markdown with embedded raw image links, commit and push changes if any, and post the comparison as a PR comment. Includes validation, debug output, and early-exit handling when no screenshots are present. --- .github/workflows/ci.yml | 2 +- .github/workflows/publish-screenshots.yml | 244 ++++++++++++++++++++-- 2 files changed, 223 insertions(+), 23 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 781f6bf..e099f0b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -212,7 +212,7 @@ jobs: if: >- always() && (steps.test.outcome == 'success' || steps.test.outcome == 'failure') - uses: actions/upload-artifact@v6 + uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 with: name: tray-screenshots-${{ runner.os }}${{ matrix.appindicator && format('-{0}', matrix.appindicator) || '' }} path: build/tests/screenshots diff --git a/.github/workflows/publish-screenshots.yml b/.github/workflows/publish-screenshots.yml index efa0678..c54d843 100644 --- a/.github/workflows/publish-screenshots.yml +++ b/.github/workflows/publish-screenshots.yml @@ -8,6 +8,7 @@ on: - completed permissions: + actions: read contents: write pull-requests: write @@ -17,42 +18,241 @@ jobs: runs-on: ubuntu-latest steps: - name: Download Artifacts - uses: actions/download-artifact@v7 + uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0 with: - path: screenshots - pattern: tray-screenshots-* + github-token: ${{ secrets.GITHUB_TOKEN }} + repository: ${{ github.repository }} run-id: ${{ github.event.workflow_run.id }} + path: run-screenshots + pattern: tray-screenshots-* + + - name: Prepare Matrix Screenshot Directories + run: | + mkdir -p prepared - - name: Debug screenshots - run: ls -R screenshots + shopt -s nullglob + for artifact_dir in run-screenshots/tray-screenshots-*; do + [ -d "${artifact_dir}" ] || continue + matrix_name="${artifact_dir##*/tray-screenshots-}" + mkdir -p "prepared/${matrix_name}" + cp -R "${artifact_dir}/." "prepared/${matrix_name}/" + done - - name: Determine Branch and Path - id: determine + if [ -z "$(find prepared -mindepth 1 -print -quit)" ]; then + echo "No screenshots were downloaded from CI artifacts." + exit 1 + fi + + echo "Prepared screenshot files:" + find prepared -type f | sort + + - name: Determine Context + id: context env: - PULL_REQUEST_NUMBER: ${{ github.event.workflow_run.pull_requests[0].number }} - HEAD_BRANCH: ${{ github.event.workflow_run.head_branch }} + WORKFLOW_EVENT: ${{ github.event.workflow_run.event }} + WORKFLOW_HEAD_BRANCH: ${{ github.event.workflow_run.head_branch }} run: | - if [ -n "${PULL_REQUEST_NUMBER}" ]; then - PR_NUMBER=${PULL_REQUEST_NUMBER} - BRANCH_PATH="PR-${PULL_REQUEST_NUMBER}" + event_name="${WORKFLOW_EVENT}" + head_branch="${WORKFLOW_HEAD_BRANCH}" + pr_number="$(jq -r '.workflow_run.pull_requests[0].number // empty' "${GITHUB_EVENT_PATH}")" + + if [ -n "${pr_number}" ] && ! [[ "${pr_number}" =~ ^[0-9]+$ ]]; then + echo "Invalid pr_number value: ${pr_number}" + exit 1 + fi + + is_pr=false + if [ "${event_name}" = "pull_request" ] && [ -n "${pr_number}" ]; then is_pr=true - else - BRANCH_NAME=$(echo "${HEAD_BRANCH}" | sed 's/\//-/g') - BRANCH_PATH="${BRANCH_NAME}" - is_pr=false + fi + + is_master_push=false + if [ "${event_name}" = "push" ] && [ "${head_branch}" = "master" ]; then + is_master_push=true fi { - echo "branch_path=${BRANCH_PATH}" + echo "event_name=${event_name}" + echo "head_branch=${head_branch}" echo "is_pr=${is_pr}" - echo "pr_number=${PR_NUMBER}" + echo "pr_number=${pr_number}" + echo "is_master_push=${is_master_push}" } >> "${GITHUB_OUTPUT}" - # debug outputs - cat "${GITHUB_OUTPUT}" - - name: Checkout Screenshots Branch - uses: actions/checkout@v6 + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: ref: screenshots path: screenshots-repo + + - name: Sync Screenshot Content + id: sync + if: steps.context.outputs.is_master_push == 'true' || steps.context.outputs.is_pr == 'true' + env: + PR_NUMBER: ${{ steps.context.outputs.pr_number }} + run: | + target_dir="" + + if [ "${{ steps.context.outputs.is_master_push }}" = "true" ]; then + target_dir="screenshots-repo/baseline" + elif [ "${{ steps.context.outputs.is_pr }}" = "true" ]; then + if ! [[ "${PR_NUMBER}" =~ ^[0-9]+$ ]]; then + echo "Invalid PR number: ${PR_NUMBER}" + exit 1 + fi + target_dir="screenshots-repo/pull-requests/PR-${PR_NUMBER}" + else + echo "Unsupported workflow context for screenshot sync." + exit 1 + fi + + mkdir -p "${target_dir}" + + # Mirror the prepared set and delete files removed from CI output. + rsync -a --delete prepared/ "${target_dir}/" + + echo "target_dir=${target_dir}" >> "${GITHUB_OUTPUT}" + + - name: Build PR Screenshot Comparison Comment + if: steps.context.outputs.is_pr == 'true' + env: + REPOSITORY: ${{ github.repository }} + PR_NUMBER: ${{ steps.context.outputs.pr_number }} + BASELINE_ROOT: screenshots-repo/baseline + PR_ROOT: prepared + run: | + raw_base="https://raw.githubusercontent.com/${REPOSITORY}/screenshots" + baseline_root="${BASELINE_ROOT}" + pr_root="${PR_ROOT}" + + { + echo "## Screenshot Comparison" + echo + printf "PR #%s screenshots vs \`screenshots\` baseline.\n\n" "${PR_NUMBER}" + } > pr-comment.md + + tmp_baseline="$(mktemp)" + tmp_pr="$(mktemp)" + tmp_matrices="$(mktemp)" + + if [ -d "${baseline_root}" ]; then + find "${baseline_root}" -mindepth 1 -maxdepth 1 -type d -printf '%f\n' >> "${tmp_matrices}" + fi + if [ -d "${pr_root}" ]; then + find "${pr_root}" -mindepth 1 -maxdepth 1 -type d -printf '%f\n' >> "${tmp_matrices}" + fi + + if [ ! -s "${tmp_matrices}" ]; then + echo "No matrix screenshots were found in baseline or PR artifacts." >> pr-comment.md + rm -f "${tmp_baseline}" "${tmp_pr}" "${tmp_matrices}" + exit 0 + fi + + while IFS= read -r matrix; do + [ -n "${matrix}" ] || continue + + baseline_matrix="${baseline_root}/${matrix}" + pr_matrix="${pr_root}/${matrix}" + + : > "${tmp_baseline}" + : > "${tmp_pr}" + + if [ -d "${baseline_matrix}" ]; then + ( + cd "${baseline_matrix}" + find . -type f | sed 's#^\./##' | LC_ALL=C sort + ) > "${tmp_baseline}" + fi + + if [ -d "${pr_matrix}" ]; then + ( + cd "${pr_matrix}" + find . -type f | sed 's#^\./##' | LC_ALL=C sort + ) > "${tmp_pr}" + fi + + { + printf "### Matrix: \`%s\`\n\n" "${matrix}" + echo "| Image | Baseline | PR |" + echo "| --- | --- | --- |" + } >> pr-comment.md + + tmp_all="$(mktemp)" + cat "${tmp_baseline}" "${tmp_pr}" | LC_ALL=C sort -u > "${tmp_all}" + + if [ ! -s "${tmp_all}" ]; then + echo "| _(none)_ | | |" >> pr-comment.md + echo >> pr-comment.md + rm -f "${tmp_all}" + continue + fi + + while IFS= read -r rel_file; do + [ -n "${rel_file}" ] || continue + + baseline_cell="" + if grep -Fxq "${rel_file}" "${tmp_baseline}"; then + baseline_cell="" + fi + + pr_cell="" + if grep -Fxq "${rel_file}" "${tmp_pr}"; then + pr_cell="" + fi + + printf "| \`%s\` | %s | %s |\n" "${rel_file}" "${baseline_cell}" "${pr_cell}" >> pr-comment.md + done < "${tmp_all}" + + echo >> pr-comment.md + rm -f "${tmp_all}" + done < <(LC_ALL=C sort -u "${tmp_matrices}") + + rm -f "${tmp_baseline}" "${tmp_pr}" "${tmp_matrices}" + echo "Generated pr-comment.md" + + - name: Commit and Push Screenshot Changes + id: push + if: steps.context.outputs.is_master_push == 'true' || steps.context.outputs.is_pr == 'true' + env: + PR_NUMBER: ${{ steps.context.outputs.pr_number }} + WORKFLOW_HEAD_SHA: ${{ github.event.workflow_run.head_sha }} + run: | + cd screenshots-repo + + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + + commit_message="" + if [ "${{ steps.context.outputs.is_master_push }}" = "true" ]; then + git add -A baseline + commit_message="chore: update screenshots (${WORKFLOW_HEAD_SHA})" + elif [ "${{ steps.context.outputs.is_pr }}" = "true" ]; then + if ! [[ "${PR_NUMBER}" =~ ^[0-9]+$ ]]; then + echo "Invalid PR number: ${PR_NUMBER}" + exit 1 + fi + git add -A "pull-requests/PR-${PR_NUMBER}" + commit_message="chore: update PR-${PR_NUMBER} screenshots (${WORKFLOW_HEAD_SHA})" + else + echo "Unsupported workflow context for commit/push." + exit 1 + fi + + if git diff --cached --quiet; then + echo "has_changes=false" >> "${GITHUB_OUTPUT}" + exit 0 + fi + + git commit -m "${commit_message}" + git push origin screenshots + echo "has_changes=true" >> "${GITHUB_OUTPUT}" + + - name: Post PR Comparison Comment + if: steps.context.outputs.is_pr == 'true' + uses: mshick/add-pr-comment@ffd016c7e151d97d69d21a843022fd4cd5b96fe5 # v3.9.0 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + issue: ${{ steps.context.outputs.pr_number }} + message-path: pr-comment.md + message-id: screenshot-comparison + refresh-message-position: true