search

odygrd / quill

Asynchronous Low Latency C++ Logging Library
MIT License
1.55k stars 158 forks source link

1 test fails: rotating_file_sink_index_open_mode_write_clean_up_old_files #550

Closed yurivict closed 1 month ago

yurivict commented 1 month ago

Describe the bug

44/161 Testing: rotating_file_sink_index_open_mode_write_clean_up_old_files
44/161 Test: rotating_file_sink_index_open_mode_write_clean_up_old_files
Command: "/usr/ports/devel/quill/work/.build/build/test/TEST_RotatingFileSink" "--test-case=rotating_file_sink_index_open_mode_write_clean_up_old_files"
Directory: /usr/ports/devel/quill/work/.build/test/unit_tests
"rotating_file_sink_index_open_mode_write_clean_up_old_files" start time: Sep 05 23:34 PDT
Output:
----------------------------------------------------------
[doctest] doctest version is "2.4.11"
[doctest] run with "--help" for options
===============================================================================
/usr/ports/devel/quill/work/quill-7.0.0/test/unit_tests/RotatingFileSinkTest.cpp:202:
TEST SUITE: RotatingFileSink
TEST CASE:  rotating_file_sink_index_open_mode_write_clean_up_old_files

/usr/ports/devel/quill/work/quill-7.0.0/test/unit_tests/RotatingFileSinkTest.cpp:239: FATAL ERROR: REQUIRE( !fs::exists(filename_2) ) is NOT correct!
  values: REQUIRE( false )

===============================================================================
[doctest] test cases: 1 | 0 passed | 1 failed | 20 skipped
[doctest] assertions: 7 | 6 passed | 1 failed |
[doctest] Status: FAILURE!

Environment

odygrd commented 1 month ago

hey, thanks for reporting. I can not preproduce on freebsd with gcc 13. I have also tried with clang18 but the test passes

There should be no changes there compared to the previous version.

I noticed that previous tests would leave some files without removing them which is fixed in a0bdaeda0b7b34bd0b2188f914032d952689b21d but i wouldn't expect it to fail because of that

image

Can you please maybe try to rerun it ? If it persists can you maybe skip this test or not run the test suite when installing the package ?

The tests are running as part of the CI github actions for mac/linux/windows but when installing the package it is not really required to run them. Some of them are quite comprehensive and might take some additional time. Also there are 1-2 tests that are not 100% stable

yurivict commented 1 month ago

When the system is loaded some tests fail intermittently. This likely means that race conditions exist.

image

odygrd commented 1 month ago

There are few tests making some assumptions. This is one of those tests expecting a loop to run N times but it won’t run N times if the system is busy. They are mainly designed to run on github actions or the user running them manually

odygrd commented 1 month ago

‘multi_frontend_threads’ and ‘user_sink’ tests might be unstable depending on the system load. The reason for the former is that we can take a timestamp and then block on a full queue for longer than expected and since we do not retake the timestamp while retrying it can lead to seeing an out of order timestamp in the log file.

The rest should be stable, not sure how the RotatingFileSink one failed as i haven’t seen it failing before

odygrd commented 1 month ago

I have stabilised those tests in https://github.com/odygrd/quill/pull/551 I tested on freebsd with stress --cpu 2000 --timeout 120 & and they are passing on my VM Will be in release 7.1.0 in a few days. If you have any issues after this release let me know