-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
task: add tests for spawn_local
and spawn_local_on
#7609
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
Hi @FrancescoV1985 , any update on this PR? |
|
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.
Could you try to reduce the duplicated code from test code? Also, Are the tests for TaskTracker
in the scope of this PR?
I committed the tests for TaskTracker. I still need to add comments and double check. I will pm you for some clarifications.. |
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.
Please sync changes from the base branch to fix the CI failures.
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.
Since the mod name has already indicated the scope, we can improve the test name, also change the comments.
Feel free to re-write the suggested changes.
Commit suggestions for task_join_set.rs Co-authored-by: Qi <[email protected]>
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.
Overall looks good to me, could you resolve the merge conflicts?
spawn_local
and spawn_local_on
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.
Thanks!
Fixes: #7562
Motivation
Improve the test coverage of task containers on LocalRuntime
Solution
Added new tests for JoinSet, JoinMap, TaskTracer