Conversation
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
There was a problem hiding this comment.
Pull Request Overview
This pull request replaces the existing thread testing infrastructure with a comprehensive test suite for the Hohnor framework. The changes transition from an older, simpler test structure to a modern, organized testing approach with extensive coverage across all major components.
Key changes:
- Complete replacement of old thread test files with comprehensive test suites
- Addition of test coverage for time, process, I/O, file, and core components
- Implementation of proper test organization with separate test files for each component
- Introduction of modern testing patterns with proper setup/teardown and atomic synchronization
Reviewed Changes
Copilot reviewed 140 out of 158 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/time/Timestamp_test.cpp | Comprehensive tests for timestamp functionality including construction, formatting, and arithmetic operations |
| test/time/Date_test.cpp | Tests for date handling, validation, and conversion operations |
| test/thread/*.cpp | Modern thread testing suite replacing old infrastructure with proper synchronization and lifecycle testing |
| test/process/ProcessInfo_test.cpp | Tests for process information gathering including CPU time, memory usage, and system parameters |
| test/io/*.cpp | I/O system tests covering epoll, file descriptors, and utilities |
| test/file/*.cpp | File operation tests including logging, utilities, and file handling |
| test/core/*.cpp | Core framework tests for event loops, timers, signals, and I/O handlers |
| test/common/circularbuffertest.cpp | Removal of old circular buffer test |
| test/thread/CMakeLists.txt | Removal of old CMake configuration |
| bool thread_ran = false; | ||
| Thread t1([&]() { | ||
| thread_ran = true; | ||
| CurrentThread::sleepUsec(50 ); // 50ms |
There was a problem hiding this comment.
The sleep duration is only 50 microseconds, which may be too short. The comment indicates 50ms but the value suggests 50 microseconds. Consider using 50 * 1000 for consistency with other tests.
| CurrentThread::sleepUsec(50 ); // 50ms | |
| CurrentThread::sleepUsec(50 * 1000); // 50ms |
| pool.run([&taskCount]() { | ||
| taskCount++; | ||
| // Simulate some work | ||
| volatile int dummy = 0; |
There was a problem hiding this comment.
Using 'volatile' for simple loop work is not recommended in modern C++. Consider using std::atomic or a more appropriate mechanism for preventing optimization.
| volatile int dummy = 0; | |
| std::atomic<int> dummy(0); |
| TEST_F(FdGuardTest, NonCopyable) { | ||
| // FdGuard should inherit from NonCopyable | ||
| // This is a compile-time test - if it compiles, the test passes | ||
| static_assert(std::is_base_of<NonCopyable, FdGuard>::value, "Epoll must derive from FdGuard"); |
There was a problem hiding this comment.
The static_assert error message says "Epoll must derive from FdGuard" but should say "FdGuard must derive from NonCopyable" to match the actual assertion being tested.
| static_assert(std::is_base_of<NonCopyable, FdGuard>::value, "Epoll must derive from FdGuard"); | |
| static_assert(std::is_base_of<NonCopyable, FdGuard>::value, "FdGuard must derive from NonCopyable"); |
No description provided.