Merged
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #86 +/- ##
===========================================
+ Coverage 62.01% 73.22% +11.20%
===========================================
Files 4 4
Lines 337 422 +85
Branches 63 81 +18
===========================================
+ Hits 209 309 +100
+ Misses 92 70 -22
- Partials 36 43 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
|
ReenigneArcher
force-pushed
the
test/add-screenshots
branch
29 times, most recently
from
d361a0c to
c6d4b9b
Compare
ReenigneArcher
force-pushed
the
test/add-screenshots
branch
5 times, most recently
from
e9ed38e to
cd13799
Compare
Member
Author
|
@ThomVanL I think I'm facing a similar problem with permissions in this PR. Wondering if you can provide some assistance. We use this library for Sunshine's tray icon. Any idea how to grant the proper permission to avoid this popup warning/error? 
|
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.
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.
Eliminated unnecessary tray_loop(1) calls in unit tests for tooltip, checkbox, and icon updates. WaitForTrayReady is sufficient for synchronization, simplifying the test logic.
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.
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.
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.
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.
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.
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.
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.
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.
Update the 'Setup virtual desktop' step in the CI workflow to use LizardByte/actions@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.
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.
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.
|
@ReenigneArcher I think the particular permission you might be looking could be # system TCC.DB
sudo sqlite3 "/Library/Application Support/com.apple.TCC/TCC.db" "
SELECT * FROM access
WHERE service='kTCCServiceScreenCapture'"
# kTCCServiceScreenCapture|com.apple.Terminal|0|2|4|1|??^L||0|UNUSED||0|1774218150|||UNUSED|1774218150
# kTCCServiceScreenCapture|com.microsoft.VSCode|0|2|4|1|??^L||0|UNUSED||0|1774217534|||UNUSED|1774217534
# user TCC.DB
sudo sqlite3 "$HOME/Library/Application Support/com.apple.TCC/TCC.db" "
SELECT * FROM access
WHERE service='kTCCServiceScreenCapture'"
# <empty>Just based off this, I think that particular permission needs to be in the system TCC database, not the one under the user's home. You might already know this but accessing the TCC databases also requires your process to have "Full Disk Access" permissions, but that should already be the case with Github runners (though I'm not entirely sure if you're using those). If you're looking to add a client with an absolute path, then the record structure changes slightly and you could use the script I referenced for my core audio tap PR. |
|
So to be more complete... I think this might help: set -euo pipefail
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"
}
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"
}
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
userValuesArray=(
"'kTCCServiceScreenCapture','/bin/bash',1,2,0,1,NULL,NULL,NULL,'UNUSED',NULL,0,1583997993"
)
for values in "${userValuesArray[@]}"; do
configure_user_tccdb "$values,NULL,NULL,'UNUSED',${values##*,}"
done
echo "macOS TCC permissions configured." |
Member
Author
|
@ThomVanL thank you! I will give your suggestion a try. I am just using GitHub's hosted runners, nothing self hosted. Edit: still giving me the same popup |
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>
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-<n>) 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.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR adds screenshots to tests. The goal is to be able to easily review what the tray icon looks like in different states.
TODO:
Screenshot
Issues Fixed or Closed
Roadmap Issues
Type of Change
Checklist
AI Usage