Skip to content

Fix thread safety issues in UnitTests#415

Merged
ZachNagengast merged 3 commits intoargmaxinc:swift-6from
naykutguven:unit-test-concurrency-fix
Feb 23, 2026
Merged

Fix thread safety issues in UnitTests#415
ZachNagengast merged 3 commits intoargmaxinc:swift-6from
naykutguven:unit-test-concurrency-fix

Conversation

@naykutguven
Copy link
Contributor

This pull request improves the thread safety and reliability of the segmentDiscoveryCallback test in UnitTests.swift by introducing a lock to protect shared state. Additionally, it makes a minor import update and refactors how the audio file path is retrieved.

Thread safety improvements:

  • Replaced the plain array discoveredSegments with an OSAllocatedUnfairLock-protected array to ensure thread-safe access when segments are discovered and appended in the callback. All subsequent accesses to discoveredSegments are now done through the lock.

General refactoring and minor changes:

  • Moved the retrieval of the audioFilePath to earlier in the test setup for clarity and to avoid redundant code.
  • Added the os.lock import to support the new locking mechanism.

Copilot AI review requested due to automatic review settings February 14, 2026 15:36
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves reliability of callback-driven unit tests by removing a data race in the segment discovery callback and slightly refactoring test setup for clarity.

Changes:

  • Added os.lock import and used OSAllocatedUnfairLock to protect the shared discoveredSegments collection in the VAD callback test.
  • Refactored audioFilePath retrieval in testCallbackWithEarlyStopping() to happen earlier in the test setup.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@naykutguven naykutguven force-pushed the unit-test-concurrency-fix branch from 9e56a90 to 6fc544e Compare February 23, 2026 17:45
@ZachNagengast ZachNagengast merged commit 2d78b36 into argmaxinc:swift-6 Feb 23, 2026
9 checks passed
@naykutguven naykutguven deleted the unit-test-concurrency-fix branch February 23, 2026 19:36
naykutguven added a commit to naykutguven/WhisperKit that referenced this pull request Mar 3, 2026
* Fix thread safety issues in UnitTests

* Fix thread safety error in TestUtils

* Move helper class inside the util method
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants