KF-30 add tests for EResource #62
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive test coverage for the EResource class, implementing unit tests to verify exclusive and shared locking mechanisms, lock conversions, and multi-threaded scenarios.
- Adds test scenarios for basic exclusive and shared resource acquisition and release
- Tests lock conversion functionality from exclusive to shared locks
- Verifies multi-threaded behavior to ensure proper synchronization and deadlock prevention
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| test/EResourceTest.cpp | Comprehensive test suite covering EResource functionality including basic locking, conversions, and multi-threaded scenarios |
| test/CMakeLists.txt | Added EResourceTest.cpp to the build configuration |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Kellesi
force-pushed
the
KF-30-test-EResource
branch
from
5a7d4ee to
a058a22
Compare
Kellesi
force-pushed
the
KF-30-test-EResource
branch
from
a058a22 to
f46b806
Compare
test/EResourceTest.cpp
Outdated
|
|
||
| WHEN("The resource is converted to shared") | ||
| { | ||
| bool exclusiveAcquired = resource.acquireExclusive(); |
There was a problem hiding this comment.
these 2 lines should be in GIVEN
test/EResourceTest.cpp
Outdated
| } | ||
| } | ||
|
|
||
| GIVEN("The EResource and 2 threads") |
There was a problem hiding this comment.
@SergiusTheBest I think it's time to introduce the kf-benchmark project, it is already added in my old branch, I can create a PR from it immediately
| } | ||
| } | ||
|
|
||
| WHEN("Multiple threads attempt to acquire shared the resource") |
There was a problem hiding this comment.
There is a better way to check that shared access was successfull without waiting. Create a hundred of threads that read some resource and register this event, then join them all and then check that every consumer got what it wanted
There was a problem hiding this comment.
added ThreadPool with maximum count of logical threads
test/EResourceTest.cpp
Outdated
| #include <kf/Thread.h> | ||
|
|
||
| #include <kf/ThreadPool.h> | ||
| #include <kf/stl/vector> |
There was a problem hiding this comment.
please add 1 more new line
There was a problem hiding this comment.
++
removed redundant include of vector
| LONG counter = 0; | ||
| }; | ||
| kf::EResource resource; | ||
| std::array<bool, kMaxThreadsCount> acquired; |
There was a problem hiding this comment.
Check that numLogicalProcessors <= kMaxThreadsCount
There was a problem hiding this comment.
KeQueryActiveProcessorCount returns a number of processors for one processor group, which is a static set of up to 64 logical processors. So, the return value can never be greater than 64.
https://learn.microsoft.com/en-us/windows/win32/procthread/processor-groups#:~:text=Support%20for%20systems,group%2C%20Group%200.
Even if it could, the ThreadPool constructor explicitly caps the number of worker threads at 64
Task: https://jira.dev.local/jira/browse/KF-30