-
Notifications
You must be signed in to change notification settings - Fork 115
Исправлена ошибка с тем что в фоновых заданиях не срабатывали подписки на события #1489
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe changes address an issue with event handlers not being triggered in background tasks. The modifications involve updating the Changes
Sequence DiagramsequenceDiagram
participant BG as Background Task
participant MI as MachineInstance
participant EP as EventProcessor
BG->>MI: Capture Current EventProcessor
MI-->>BG: Return EventProcessor
BG->>MI: Set EventProcessor Context
BG->>EP: Trigger Event
EP->>BG: Execute Event Handlers
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/tasks.os (1)
274-294: LGTM! Well-structured test with proper cleanup.The test follows good practices:
- Clear Arrange-Act-Assert pattern
- Proper cleanup by unsubscribing from the event
- Descriptive assertion message
Consider enhancing the test to verify that the event handler is actually called in the background thread context.
Процедура ТестДолжен_ПроверитьЧтоОбработчикиСобытийВызываютсяВФоновомЗадании() Экспорт СобытиеВызвано = Ложь; + ВызваноВФоновомПотоке = Ложь; Параметры = Новый Массив; Параметры.Добавить("ЯСобытие"); Параметры.Добавить(Новый Массив); ДобавитьОбработчик ЭтотОбъект.ЯСобытие, Событие; ФоновоеЗадание = ФоновыеЗадания.Выполнить(ЭтотОбъект, "ВызватьСобытие", Параметры); ФоновоеЗадание.ОжидатьЗавершения(); УдалитьОбработчик ЭтотОбъект.ЯСобытие, Событие; юТест.ПроверитьИстину( СобытиеВызвано, "Ожидали что при вызове события в фоновом задании вызовется, а это не так" ); + юТест.ПроверитьИстину( + ВызваноВФоновомПотоке, + "Ожидали что событие вызовется в фоновом потоке" + ); КонецПроцедуры
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/OneScript.StandardLibrary/Tasks/BackgroundTasksManager.cs(1 hunks)tests/tasks.os(4 hunks)
🔇 Additional comments (4)
src/OneScript.StandardLibrary/Tasks/BackgroundTasksManager.cs (1)
50-56: LGTM! Clean fix for event handling in background tasks.The implementation correctly preserves the EventProcessor context across thread boundaries by capturing it before task creation and restoring it within the worker task. This ensures event handlers work properly in background tasks.
tests/tasks.os (3)
6-6: LGTM! Clear and purposeful variable declaration.The variable name clearly indicates its purpose of tracking event handler invocation.
27-27: LGTM! Test case name is descriptive and follows conventions.The test case name clearly communicates its purpose of verifying event handler invocation in background tasks.
74-76: LGTM! Simple and focused event handler implementation.The event handler is correctly exported and has a clear, single responsibility of setting the event flag.
Исправлена ошибка с тем что в фоновых заданиях не срабатывали подписки на события
closes: #1485
Summary by CodeRabbit
Tests
Bug Fixes