Repository navigation
test: time out a job that cannot finish first instead of racing a 1ms timeout - #63
Project516 wants to merge 1 commit into
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 1 minute. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Replaces the 1ms-timeout / 1s-clip timeout tests in three test files with a 50ms timeout against an hour-long lavfi testsrc job writing to null, so the job cannot finish before the timeout fires on either the st or mt core. The old 1ms timeout raced the cooperative st scheduler, which is why the node test flaked once on the #41 merge.
review-bot, model qwen/qwen3.8-27b:free, verdict approve
|
Closing. A long running job with a short timeout is deterministic on st, but on the mt core the exec() after it fails with |
What
The timeout tests (
tests/ffmpeg-core.test.js,tests/ffmpeg.test.js,tests/ffmpeg-node.test.mjs) now run a long lavfi job (testsrcfor an hour, to-f null) with a 50ms timeout instead of a 1s transcode with a 1ms timeout.Why
The old test raced the job. The st core is cooperative, so the main fiber that polls
is_timeout()only runs when the other fibers block, and a one-second clip can finish before it does.node-testsfailed once on the #41 merge to master withexpected +0 to equal 1and passed on every earlier run.release.ymlruns the whole CI workflow as its gate, so a flaky test blocks publishing. A job that cannot finish first makes the result deterministic on both cores.Testing
CI on this PR.