-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Fix the order of callback by performFunctionInCocosThread and add unit-test for it. #17543
Conversation
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.
LGTM
2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
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.
LGTM
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
Interface Check ReportThis pull request does not change any public interfaces ! |
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.
LGTM
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
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.
LGTM
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
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.
LGTM
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
[ RUN ] schedulerTest.performInCocosThreadOrder unknown file: error: SEH exception with code 0xc0000005 thrown in the test body. [ FAILED ] schedulerTest.performInCocosThreadOrder (0 ms)
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.
25 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings
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.
logic: Removing all unit tests for path utilities may reduce test coverage. Ensure these tests are either no longer needed or have been moved elsewhere.
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.
logic: Removing all geometry tests may reduce test coverage. Ensure these tests are either replaced or no longer necessary.
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.
logic: Removing all Mat3 unit tests may reduce code coverage and make it harder to catch regressions in future changes to the Mat3 class. Ensure these tests are either relocated or replaced with equivalent tests elsewhere.
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.
logic: Removing all math utility tests may reduce test coverage for critical mathematical operations. Ensure these tests are either relocated or replaced with equivalent tests elsewhere.
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.
26 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings
TEST(schedulerTest, performInCocosThreadOrder) { | ||
auto scheduler = std::make_shared<Scheduler>(); | ||
|
||
std::vector<int> orderResult; |
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.
style: Consider using a thread-safe container for orderResult to prevent potential race conditions
Re: #17012, #17464
Changelog
Continuous Integration
This pull request:
Compatibility Check
This pull request:
Greptile Summary
This pull request modifies the
performInCocosThreadOrder
test innative/tests/unit-test/src/scheduler_test.cpp
to improve the validation of task execution order in the Scheduler.std::shared_ptr<Scheduler>
for better memory managementEXPECT_EQ
toEXPECT_TRUE
with direct vector comparison