-
Notifications
You must be signed in to change notification settings - Fork 53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add tests for find_recent_logfile()
#2391
Add tests for find_recent_logfile()
#2391
Conversation
Can one of the admins verify this patch? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2391 +/- ##
==========================================
+ Coverage 98.80% 99.12% +0.32%
==========================================
Files 84 84
Lines 3424 3424
==========================================
+ Hits 3383 3394 +11
+ Misses 41 30 -11 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mikeweltevrede: Thank you very much for taking the time to write tests for this function, which in fact discovered a bug! That's why we write thorough tests, and I appreciate your attention to detail with this.
I would be happy to merge this PR once my one comment is addressed. Don't hesitate to let me know if you have any questions.
Fantastic, thank you @mikeweltevrede! I appreciate your help with this. This is wonderful. I am happy to merge. |
@Andrew-S-Rosen no problem, happy to have been able to assist :) |
Summary of Changes
Add tests for
find_recent_logfile()
to close #2389. Note that I also noticed that there might be some unintended functionality (seetest_find_recent_logfile_only_checks_files_matching_the_extension
) and added a test that is expected to fail.Requirements
main
).Note: If you are an external contributor, you will see a comment from @buildbot-princeton. This is solely for the maintainers.