Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Improve logger test #5419

Merged
merged 2 commits into from
Oct 4, 2023
Merged

Improve logger test #5419

merged 2 commits into from
Oct 4, 2023

Conversation

jenshnielsen
Copy link
Collaborator

  • When possible avoid reading from log file but use caplog to get logging
  • Rework cleanup fixture to not touch pytest handlers but only remove handlers inserted by the test
  • Ensure that logging is off by default in tests

Closes #5418

@jenshnielsen
Rework log test
60df462 
Read setup log from caplog fixture rather than file.
Remove the need for setting / resetting loggers in tests
that do not require it.
@jenshnielsen
Rework logging fixtures to cleanup after test
65cff6a 
This avoids modifying the pytest fixtures which has unintended
side effects
@jenshnielsen jenshnielsen enabled auto-merge October 4, 2023 09:32
@codecov
Copy link

codecov bot commented Oct 4, 2023 •
edited
Loading

Codecov Report

Merging #5419 (65cff6a) into master (5525025) will decrease coverage by 0.01%.
Report is 2 commits behind head on master.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #5419      +/-   ##
==========================================
- Coverage   68.27%   68.27%   -0.01%     
==========================================
  Files         341      341              
  Lines       29901    29901              
==========================================
- Hits        20415    20414       -1     
- Misses       9486     9487       +1

@jenshnielsen jenshnielsen added this pull request to the merge queue Oct 4, 2023
Merged via the queue into microsoft:master with commit 30659dd Oct 4, 2023
@jenshnielsen jenshnielsen deleted the improve_logger_test branch October 4, 2023 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@astafan8 astafan8 astafan8 approved these changes

@Dominik-Vogel Dominik-Vogel Awaiting requested review from Dominik-Vogel

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Random error in test_logger
2 participants
@jenshnielsen @astafan8