Extract logging tests to a separate executable.#2087
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #2087 +/- ##
========================================
Coverage 82.69% 82.69%
========================================
Files 332 333 +1
Lines 59700 59710 +10
Branches 12267 12090 -177
========================================
+ Hits 49368 49377 +9
+ Misses 8928 8924 -4
- Partials 1404 1409 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Well, this is inconvenient. I reworked the multithreaded test to actually stress test the logger for synchronization issues, and apparently |
| void printLogThread(int threadId, int numMessages) | ||
| { | ||
| std::random_device rd; | ||
| std::mt19937 simpleRand(rd()); | ||
| std::uniform_int_distribution<int> dist(1, 5); | ||
|
|
||
| std::ostringstream sstream; | ||
| sstream << threadId; | ||
| std::string threadIdAsString = sstream.str(); | ||
| for (int i = 0; i < 1000; i++) | ||
|
|
||
| for (int i = 0; i < numMessages; i++) | ||
| { | ||
| pcpp::invokeErrorLog(threadIdAsString); | ||
|
|
||
| int sleepTime = dist(simpleRand); | ||
| std::this_thread::sleep_for(std::chrono::milliseconds(sleepTime)); | ||
| } | ||
| } |
There was a problem hiding this comment.
This test is faulty as it does not test multithreaded calls reliably.
The current implementation does test calling the logger from different threads, but does not reliably perform them simultaneously due to the large sleep intervals. The chance of Thread1 and Thread2 calling the logger at the same time to test for actual sync issues is exceedingly rare.
It is also a large time sink in the CI pipeline, due to the test "faking" work by sleeping a lot.
Originally the PR included fixes for this, but I decided to separate those to keep the PR focused.
| @@ -34,66 +35,86 @@ namespace pcpp | |||
| class LogPrinter | |||
There was a problem hiding this comment.
Converted LogPrinter to an instance to simplify the code as the logger can now take a lambda callback.
1 participant
Fixes #2083
Bug
After the logger tests are ran, the logger configuration is restored to its default state instead of to the state it was prior to the test being ran.
Proposed fix
A proposed fix is to extract all logger tests to a separate executable. This is because the logger tests modify the logger configuration and callback, The callback specifically cannot be extracted from the logger, so a better solution would be to separate those tests in a new program.