google / glog

C++ implementation of the Google logging module
http://google.github.io/glog/
BSD 3-Clause "New" or "Revised" License
6.99k stars 2.05k forks source link

Fixed IsLogFromCurrentProject to include all severities #1062

Closed Mivekk closed 2 months ago

Mivekk commented 7 months ago

Previously when executing following code LogCleaner would only delete overdue Error log files:

google::EnableLogCleaner(1min);

LOG(ERROR) << "Error hello";
LOG(INFO) << "Info hello";
LOG(WARNING) << "Warning hello";

And if we changed the order to for example:

LOG(INFO) << "Info hello";
LOG(ERROR) << "Error hello";
LOG(WARNING) << "Warning hello";

LogCleaner would only delete overdue Info log files and overdue Error and Warning files were ignored. To include all severities during LogCleaner's sweep I tested filepath for all severities in a IsLogFromCurrentProject function.

Mivekk commented 7 months ago

I'm not sure how I should modify cleaner test cases unfortunately

sergiud commented 7 months ago

You can extend any of the cleanup_*_unittest.cc, for instance, here https://github.com/google/glog/blob/ac12a9e79440a2a89cc1c5617bb8f6db02035182/src/cleanup_immediately_unittest.cc#L62-L64 by additionally logging at the missing severities.

Please note that the unit tests need to be updated either way because these are failing now with your changes.

Mivekk commented 7 months ago

Updated unit test and all checks should pass now hopefully

sergiud commented 7 months ago

Unfortunately, the cleanup tests are still failing.

Mivekk commented 7 months ago

Fixed an issue different way

sergiud commented 7 months ago

The unit tests are still failing though.

sergiud commented 2 months ago

Closing as abandoned. Please let me know if you want to continue working on this PR.