Skip to content

gh-157161: Fix test_rollover_based_on_st_birthtime_only on slow machines #157162

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

Open
iamsharduld wants to merge 3 commits into
base: main
Choose a base branch
from
Open

gh-157161: Fix test_rollover_based_on_st_birthtime_only on slow machines #157162

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
34d838a
gh-157161: Fix test_rollover_based_on_st_birthtime_only on slow machines
iamsharduld Sep 8, 2026
dffb770
Address review: name the margin and explain the two additions
iamsharduld Sep 9, 2026
4e22320
Remove the NEWS entry
iamsharduld Sep 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions Lib/test/test_logging.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6752,6 +6752,7 @@ def add_record(message: str) -> None:
fh.emit(record)
fh.close()

start = datetime.datetime.now()
add_record('testing - initial')
self.assertLogFile(self.fn)
# Sleep a little over the half of rollover time - and this value
Expand All @@ -6764,18 +6765,21 @@ def add_record(message: str) -> None:

# At this point, the log file should be rotated if the rotation
# is based on creation time but should be not if it's based on
# modification time.
# modification time. The rotated file is named after the creation
# time of the log file, so look back over the whole duration of the
# test, plus a margin: the file was created in setUp(), and the
# names have a resolution of one second.
found = False
now = datetime.datetime.now()
GO_BACK = 5 # seconds
for secs in range(GO_BACK + 1):
go_back = int((now - start).total_seconds()) + 2

@brittanyrey brittanyrey Sep 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to include an inline comment on how this number is derived?
edit: or maybe rename s/go_back/total_time_spent (total_test_time, test_duration)

@iamsharduld iamsharduld Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, done. The window is now built in two named steps: test_duration from the measured elapsed time, then oldest = test_duration + 2, with a comment saying where the two seconds come from (the log file is created in setUp(), a moment before the test starts, and the suffix has a one second resolution).

for secs in range(go_back + 1):

@brittanyrey brittanyrey Sep 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why add 2 on the line above, just to add 1 here?

@iamsharduld iamsharduld Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They were doing two different jobs, which is exactly why it read badly. The + 2 is the margin described above; the + 1 only makes range() cover oldest itself, since it is exclusive. The margin now has its own named variable and the loop carries an # inclusive of oldest comment, so neither number is bare arithmetic any more.

prev = now - datetime.timedelta(seconds=secs)
fn = self.fn + prev.strftime(".%Y-%m-%d_%H-%M-%S")
found = os.path.exists(fn)
if found:
self.rmfiles.append(fn)
break
msg = 'No rotated files found, went back %d seconds' % GO_BACK
msg = 'No rotated files found, went back %d seconds' % go_back
if not found:
# print additional diagnostics
dn, fn = os.path.split(self.fn)
Expand Down
3 changes: 3 additions & 0 deletions Misc/NEWS.d/next/Tests/2026-09-08-03-20-00.gh-issue-157161.SGacVD.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix ``test_rollover_based_on_st_birthtime_only`` in ``test_logging`` on slow
machines: search for the rotated file over the whole duration of the test
instead of a fixed 5 seconds.
Loading