Skip to content

Commit e076193

Browse files
committed
MacOS flaky tests
* enable recording, streaming, dual-output tests on MacOS * set CI env var for MacOS github runner for obs_handler.ts * fix warning we cant assign null: `let secondContext: osn.IVideo = null;` * do not invoke `AddVideoContext` on CI builds in enhanced broadcasting tests since these are disabled. * main.yml: upgrade to macos-15 for arm64 ARCH (same OS Intel ARCH is using). * main.yml: upload OBS logs to troubleshoot unit tests. main.yml: set fail-fast to false * turn off fail-fast so the tests can continue running which will fix issue where if macos-15-intel fails, macos-arm64 is cancelled before it can finish running. osn_handler: log time delta for long duration * add warning when signal received took longer then expectedDeadline obs_handler: assign explicit log filename for tests Fix orphaned worker thread: The WorkerSignals::worker keeps polling Query. When a test fails before calling destroy(), the server-side recording gets freed when OBS shuts down, but the orphaned worker thread continues. When the next test file creates a fresh OBS connection, Controller::GetInstance().GetConnection() now returns the new session's connection. The orphaned worker sends Query(oldUID) to the new server, which has no knowledge of that uid → "Recording reference is not valid." Remove source message listener when test ends * stops worker thread properly * test_osn_simple_recording: remove lamda to enable timeout Tests: simple_recording,test_osn_advanced_replayBuffer,test_osn_advanced_recording,dual_output - wrap with try/finally * update any test that starts a worker thread via create() to properly invoke destroy() within a finally block to guanrantee the worker thread is stopped even if the test fails. Prevents orphaned worker threads from invoking Query() every 33ms on the new IPC connection created by other tests. guarantees cleanup even when tests throw so they tests will clean up proactively, and any orphaned workers that slip through will stop themselves rather than polluting subsequent test runs. validate totalSleepMS does not have a negative value When the IPC call takes longer than sleepIntervalMS on an overloaded CI runner), sleepIntervalMS - dur.count() produces a negative result that wraps to a huge size_t value, causing the worker to sleep for an effectively infinite duration. After that, no more signals are ever polled — any test waiting for a signal hits the 30-second timeout. do not share stdout/stderr with parent process On macOS, the server is spawned with posix_spawnp with NULL for file_actions, which means it inherits the parent's stdout/stderr. On Windows this doesn't happen because CreateProcessW uses DETACHED_PROCESS | CREATE_NO_WINDOW, which cuts the server off from the parent's console entirely. obs_handler: add more retryable timeouts for flaky tests advanced-recording fix * Removed extraneous set_video_mix since audio encoders don't have a video mix which produced the "encoder 'track1' is not a video encoder" warning * Reduce defaultVideoContext from 1280x720@60fps to 1280x720@30fps to reduce CPU load on the slow x86_64 CI machine. (MacOS) controller.cpp: kill prev obs64 before starting * Should make running multiple tests in succession on this slow CI more reliable. Add a waitpid(pids[i], nullptr, 0) call that blocks until the SIGKILLed process is fully reaped by the kernel before the new posix_spawnp runs, eliminating the race. osn-replay-buffer: invoke output handler to save * makes test more reliable * follows similar pattern used by OBS::ReplayBufferSave
1 parent ce99338 commit e076193

22 files changed

Lines changed: 1465 additions & 1547 deletions

.github/workflows/main.yml

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ jobs:
4242
ReleaseName: release
4343
Architecture: x86_64
4444
- BuildReleases: Release-arm64
45-
image: macos-14
45+
image: macos-15
4646
BuildConfig: RelWithDebInfo
4747
ReleaseName: release
4848
Architecture: arm64
@@ -99,6 +99,7 @@ jobs:
9999
needs: build-macos
100100
runs-on: ${{ matrix.image }}
101101
strategy:
102+
fail-fast: false # Don't cancel the other architecture's tests if one fails, so we can get test results for both.
102103
matrix:
103104
BuildReleases: [Release-x86_64, Release-arm64]
104105
include:
@@ -108,7 +109,7 @@ jobs:
108109
ReleaseName: release
109110
Architecture: x86_64
110111
- BuildReleases: Release-arm64
111-
image: macos-14
112+
image: macos-15
112113
BuildConfig: RelWithDebInfo
113114
ReleaseName: release
114115
Architecture: arm64
@@ -151,6 +152,15 @@ jobs:
151152
OSN_ACCESS_KEY_ID: ${{secrets.AWS_RELEASE_ACCESS_KEY_ID}}
152153
OSN_SECRET_ACCESS_KEY: ${{secrets.AWS_RELEASE_SECRET_ACCESS_KEY}}
153154
RELEASE_NAME: ${{matrix.ReleaseName}}
155+
CI: true
156+
SUPPRESS_STREAMLABS_OBS_LOGS: true # Prevents logs from being printed in the test output, but still generates log files that can be uploaded as artifacts.
157+
- name: Upload OBS logs
158+
if: ${{ always() }}
159+
uses: actions/upload-artifact@v4
160+
with:
161+
name: obs-logs-mac-${{ matrix.Architecture }}
162+
path: tests/osn-tests/osnData/slobs-client/node-obs/logs/
163+
if-no-files-found: ignore
154164
# Run even after test failures so the PR still gets the flaky summary.
155165
- name: Publish flaky test check
156166
if: ${{ always() }}
@@ -181,7 +191,7 @@ jobs:
181191
ReleaseName: release
182192
Architecture: x86_64
183193
- BuildReleases: Release-arm64
184-
image: macos-14
194+
image: macos-15
185195
BuildConfig: RelWithDebInfo
186196
ReleaseName: release
187197
Architecture: arm64
@@ -234,7 +244,7 @@ jobs:
234244
ReleaseName: release
235245
Architecture: x86_64
236246
- BuildReleases: Release-arm64
237-
image: macos-14
247+
image: macos-15
238248
BuildConfig: RelWithDebInfo
239249
ReleaseName: release
240250
Architecture: arm64
@@ -348,6 +358,13 @@ jobs:
348358
OSN_ACCESS_KEY_ID: ${{secrets.AWS_RELEASE_ACCESS_KEY_ID}}
349359
OSN_SECRET_ACCESS_KEY: ${{secrets.AWS_RELEASE_SECRET_ACCESS_KEY}}
350360
RELEASE_NAME: release
361+
- name: Upload OBS logs
362+
if: ${{ always() }}
363+
uses: actions/upload-artifact@v4
364+
with:
365+
name: obs-logs-mac-${{ matrix.Architecture }}
366+
path: tests/osn-tests/osnData/slobs-client/node-obs/logs/
367+
if-no-files-found: ignore
351368
# Run even after test failures so the PR still gets the flaky summary.
352369
- name: Publish flaky test check
353370
if: ${{ always() }}

obs-studio-client/source/controller.cpp

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ std::wstring utfWorkingDir = L"";
5353
#include <libproc.h>
5454
#include <iostream>
5555
#include <spawn.h>
56+
#include <fcntl.h>
57+
#include <unistd.h>
58+
#include <sys/wait.h>
5659
extern char **environ;
5760
#endif
5861

@@ -301,16 +304,32 @@ std::shared_ptr<ipc::client> Controller::host(const std::string &uri)
301304
int st = proc_pidinfo(pids[i], PROC_PIDTBSDINFO, 0, &proc, PROC_PIDTBSDINFO_SIZE);
302305
if (st == PROC_PIDTBSDINFO_SIZE) {
303306
if (strcmp("obs64", proc.pbi_name) == 0) {
304-
if (pids[i] != 0)
307+
if (pids[i] != 0) {
305308
kill(pids[i], SIGKILL);
309+
waitpid(pids[i], nullptr, 0);
310+
}
306311
}
307312
}
308313
}
309314

310315
pid_t pid;
311316
std::vector<const char *> argv = {"obs64", uri.c_str(), version.c_str(), serverBinaryPath.c_str(), nullptr};
312317

313-
int ret = posix_spawnp(&pid, serverBinaryPath.c_str(), NULL, NULL, const_cast<char *const *>(argv.data()), environ);
318+
const char *suppressLogsEnv = std::getenv("SUPPRESS_STREAMLABS_OBS_LOGS");
319+
int ret = 0;
320+
if (suppressLogsEnv == nullptr || strcasecmp(suppressLogsEnv, "false") == 0) {
321+
// For development, it can be helpful for process to share stdout/stderr.
322+
ret = posix_spawnp(&pid, serverBinaryPath.c_str(), NULL, NULL, const_cast<char *const *>(argv.data()), environ);
323+
} else {
324+
// Do not send the blog(s) to stdout/stderr.
325+
posix_spawn_file_actions_t file_actions;
326+
posix_spawn_file_actions_init(&file_actions);
327+
posix_spawn_file_actions_addopen(&file_actions, STDOUT_FILENO, "/dev/null", O_WRONLY, 0);
328+
posix_spawn_file_actions_addopen(&file_actions, STDERR_FILENO, "/dev/null", O_WRONLY, 0);
329+
ret = posix_spawnp(&pid, serverBinaryPath.c_str(), &file_actions, NULL, const_cast<char *const *>(argv.data()), environ);
330+
posix_spawn_file_actions_destroy(&file_actions);
331+
}
332+
314333
if (ret != 0) {
315334
std::cerr << "Could not spawn the server at " << serverBinaryPath.c_str() << " with error code: " << ret << std::endl;
316335
return nullptr;

obs-studio-client/source/nodeobs_api.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,15 @@ Napi::Value api::OBS_API_initAPI(const Napi::CallbackInfo &info)
3636
std::string language;
3737
std::string version;
3838
std::string crashserverurl;
39+
std::string logFilename;
3940

4041
ASSERT_GET_VALUE(info, info[0], language);
4142
ASSERT_GET_VALUE(info, info[1], path);
4243
ASSERT_GET_VALUE(info, info[2], version);
4344
if (info.Length() > 3)
4445
ASSERT_GET_VALUE(info, info[3], crashserverurl);
46+
if (info.Length() > 4)
47+
ASSERT_GET_VALUE(info, info[4], logFilename);
4548

4649
auto conn = GetConnection(info);
4750
if (!conn)
@@ -50,7 +53,7 @@ Napi::Value api::OBS_API_initAPI(const Napi::CallbackInfo &info)
5053
conn->set_freeze_callback(ipc_freeze_callback, path);
5154

5255
std::vector<ipc::value> response = conn->call_synchronous_helper(
53-
"API", "OBS_API_initAPI", {ipc::value(path), ipc::value(language), ipc::value(version), ipc::value(crashserverurl)});
56+
"API", "OBS_API_initAPI", {ipc::value(path), ipc::value(language), ipc::value(version), ipc::value(crashserverurl), ipc::value(logFilename)});
5457

5558
// The API init method will return a response error + graphical error
5659
// If there is a problem with the IPC the number of responses here will be zero so we must validate the

obs-studio-client/source/nodeobs_autoconfig.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,8 @@ void autoConfig::worker()
6666
do_sleep:
6767
auto tp_end = std::chrono::high_resolution_clock::now();
6868
auto dur = std::chrono::duration_cast<std::chrono::milliseconds>(tp_end - tp_start);
69-
totalSleepMS = sleepIntervalMS - dur.count();
69+
auto durCount = dur.count();
70+
totalSleepMS = durCount < sleepIntervalMS ? sleepIntervalMS - durCount : 0;
7071
std::this_thread::sleep_for(std::chrono::milliseconds(totalSleepMS));
7172
}
7273
return;

obs-studio-client/source/nodeobs_service.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,8 @@ void service::worker()
348348

349349
auto tp_end = std::chrono::high_resolution_clock::now();
350350
auto dur = std::chrono::duration_cast<std::chrono::milliseconds>(tp_end - tp_start);
351-
totalSleepMS = sleepIntervalMS - dur.count();
351+
auto durCount = dur.count();
352+
totalSleepMS = durCount < sleepIntervalMS ? sleepIntervalMS - durCount : 0;
352353
std::this_thread::sleep_for(std::chrono::milliseconds(totalSleepMS));
353354
}
354355

obs-studio-client/source/worker-signals.hpp

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,16 @@ class WorkerSignals {
8888
auto conn = Controller::GetInstance().GetConnection();
8989
if (conn) {
9090
std::vector<ipc::value> response = conn->call_synchronous_helper(name, "Query", {ipc::value(refID)});
91+
if (!response.empty()) {
92+
ErrorCode firstError = (ErrorCode)response[0].value_union.ui64;
93+
if (firstError == ErrorCode::InvalidReference) {
94+
// This typically happens if the worker thread is orphaned.
95+
std::string errorMessage = response.size() > 1 ? response[1].value_str : "";
96+
std::cout << "Worker thread exiting due to Invalid reference error encountered: " << errorMessage << std::endl;
97+
isWorkerRunning = false;
98+
break;
99+
}
100+
}
91101
if ((response.size() == 5) && signalsList.size() < maximum_signals_in_queue) {
92102
ErrorCode error = (ErrorCode)response[0].value_union.ui64;
93103
if (error == ErrorCode::Ok) {
@@ -122,7 +132,8 @@ class WorkerSignals {
122132

123133
auto tp_end = std::chrono::high_resolution_clock::now();
124134
auto dur = std::chrono::duration_cast<std::chrono::milliseconds>(tp_end - tp_start);
125-
totalSleepMS = sleepIntervalMS - dur.count();
135+
auto durCount = dur.count();
136+
totalSleepMS = durCount < sleepIntervalMS ? sleepIntervalMS - durCount : 0;
126137
std::this_thread::sleep_for(std::chrono::milliseconds(totalSleepMS));
127138
}
128139

@@ -140,4 +151,4 @@ class WorkerSignals {
140151
workerThread->join();
141152
}
142153
}
143-
};
154+
};

obs-studio-server/source/nodeobs_api.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -874,10 +874,18 @@ void OBS_API::OBS_API_initAPI(void *data, const int64_t id, const std::vector<ip
874874
std::string appdata = args[0].value_str;
875875
std::string locale = args[1].value_str;
876876
currentVersion = args[2].value_str;
877+
std::string logFilename;
878+
// Skip index 3 which is reserved for crash-handler server
879+
if (args.size() > 4) {
880+
// Parse the log filename
881+
std::ostringstream ss;
882+
ss << args[4].value_str << '-' << GenerateTimeDateFilename("txt");
883+
logFilename = ss.str();
884+
}
877885
utility::osn_current_version(currentVersion);
878886

879887
/* Logging */
880-
std::string filename = GenerateTimeDateFilename("txt");
888+
std::string filename = logFilename.size() > 0 ? logFilename : GenerateTimeDateFilename("txt");
881889
std::string log_path = appdata;
882890
log_path.append("/node-obs/logs/");
883891

obs-studio-server/source/osn-replay-buffer.cpp

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -152,19 +152,23 @@ void osn::IReplayBuffer::Query(void *data, const int64_t id, const std::vector<i
152152

153153
void osn::IReplayBuffer::Save(void *data, const int64_t id, const std::vector<ipc::value> &args, std::vector<ipc::value> &rval)
154154
{
155-
obs_enum_hotkeys(
156-
[](void *data, obs_hotkey_id id, obs_hotkey_t *key) {
157-
if (obs_hotkey_get_registerer_type(key) == OBS_HOTKEY_REGISTERER_OUTPUT) {
158-
std::string key_name = obs_hotkey_get_name(key);
159-
if (key_name.compare("ReplayBuffer.Save") == 0) {
160-
obs_hotkey_enable_callback_rerouting(true);
161-
obs_hotkey_trigger_routed_callback(id, true);
162-
}
163-
}
164-
return true;
165-
},
166-
nullptr);
155+
ReplayBuffer *replayBuffer = static_cast<ReplayBuffer *>(osn::IFileOutput::Manager::GetInstance().find(args[0].value_union.ui64));
156+
if (!replayBuffer) {
157+
PRETTY_ERROR_RETURN(ErrorCode::InvalidReference, "ReplayBuffer reference is not valid.");
158+
}
167159

160+
obs_output_t *output = replayBuffer->GetOutput();
161+
if (!output) {
162+
PRETTY_ERROR_RETURN(ErrorCode::InvalidReference, "Invalid replay buffer output.");
163+
}
164+
165+
calldata_t cd = {0};
166+
proc_handler_t *ph = obs_output_get_proc_handler(output);
167+
bool hasInvoked = proc_handler_call(ph, "save", &cd);
168+
calldata_free(&cd);
169+
170+
if (!hasInvoked)
171+
PRETTY_ERROR_RETURN(ErrorCode::NotFound, "Could not find ReplayBuffer::Save");
168172
rval.push_back(ipc::value((uint64_t)ErrorCode::Ok));
169173
AUTO_DEBUG;
170-
}
174+
}

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
"local:build": "cmake --build build --target install --config Debug",
2323
"local:clean": "rm -rf build/*",
2424
"test": "electron-mocha -t 80000 --js-flags=\"--expose-gc\" --color -r ts-node/register tests/osn-tests/src/**/*.ts --reporter tests/osn-tests/util/list-reporter.js",
25-
"test:ci": "yarn run test --retries 2"
25+
"test:ci": "yarn run test --retries 3"
2626
},
2727
"devDependencies": {
2828
"@aws-sdk/client-s3": "^3.0.0",

tests/osn-tests/src/test_nodeobs_autoconfig.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,6 @@ describe(testName, function() {
5050
});
5151

5252
it('Run autoconfig', async function() {
53-
if (obs.isDarwin()) {
54-
this.skip();
55-
}
5653
const start = performance.now();
5754
let progressInfo: IConfigProgress;
5855
let settingValue: any;

0 commit comments

Comments
 (0)