8000 Fix unit tests that use Layer::ScheduleWork to actually run an event loop · Issue #22377 · project-chip/connectedhomeip · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix unit tests that use Layer::ScheduleWork to actually run an event loop #22377

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.

Sign up for GitHub

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

Jump to bottom
Open
bzbarsky-apple opened this issue Sep 2, 2022 · 3 comments
Open

Fix unit tests that use Layer::ScheduleWork to actually run an event loop #22377

bzbarsky-apple opened this issue Sep 2, 2022 · 3 comments
Labels
v1.1

Comments

@bzbarsky-apple
Copy link
Contributor

Problem

We would like to implement Layer::ScheduleWork in terms of Layer::ScheduleLambda, but currently the latter requires the platform manager event loop to be running. And we have tests that use Layer::ScheduleWork but do not run an actual event loop and expect it to magically work.

Proposed Solution

Fix unit tests that use ScheduleWork to run an event loop, and implement ScheduleWork on top of ScheduleLambda.
@bzbarsky-apple bzbarsky-apple mentioned this issue Sep 2, 2022
Merged
@andy31415 andy31415 mentioned this issue Sep 2, 2022
Merged
@bzbarsky-apple
Copy link
Contributor Author

One issue with running the event loop: The unit tests currently do a cut down event loop thing where they run just one iteration of it, by manually doing:

#if CHIP_SYSTEM_CONFIG_USE_SOCKETS
    gSystemLayer.PrepareEvents();
    gSystemLayer.WaitForEvents();
    gSystemLayer.HandleEvents();
#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS

in src/inet/tests/TestInetCommonPosix.cpp in ServiceEvents.

Comparing this to GenericPlatformManagerImpl_POSIX<ImplClass>::_RunEventLoop, it's missing the ProcessDeviceEvents call which is not public platform manager API.

So we have public API to schedule work on the platform manager (via platform eventing), but not public API to tell it to process those events as needed.

And of course on Darwin, scheduling work would usually use the Matter work queue, but that's not spun up in most unit tests.

Trying to actually use the platform manager to run the event loop puts us in a position where the "run just one event loop iteration" thing doesn't work. So tests would need to be rewritten to actually allow the event loop to run async until the relevant callbacks happen, then do things from there, complete with issues wrt which thread things are running on.

It seems like ideally the unit tests that are not using PlatformManager would be able to interpose their own PlatformEventing implementation that does whatever they want. Or override System::Layer::Layer::ScheduleLambdaBridge to do something different. Or something else that would allow them to sanely process these events.

@tcarmelveilleux @kpschoedel @andy31415

@stale
Copy link
stale bot commented Mar 7, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

1 similar comment
@stale
Copy link
stale bot commented Oct 15, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale Stale issue or PR label Oct 15, 2023
@bzbarsky-apple bzbarsky-apple removed the stale Stale issue or PR label Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
v1.1
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@bzbarsky-apple
0