Skip to content

Ensure log truncations are at nearest newline after truncation size#62

Open
ccrighton wants to merge 2 commits intopimoroni:mainfrom
ccrighton:logging-truncate
Open

Ensure log truncations are at nearest newline after truncation size#62
ccrighton wants to merge 2 commits intopimoroni:mainfrom
ccrighton:logging-truncate

Conversation

@ccrighton
Copy link
Copy Markdown

I reviewed #41 and found that it didn't behave consistently.

The behaviour expected is to truncate the file at the new line immediately after the truncation break point is reached. This means the truncation is always at least as small as requested. There is also cases at the start and end of file in #41 that do not behave rationally.

I would recommend this PR instead of #41.

Run the test cases with:
python -m unittest tests/*_test.py -v

Here is the test case results from #41 using the unit tests from this PR:

.FFFFFFF
======================================================================
FAIL: test_truncate_after_last_newline (__main__.TestUtils)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/charlie/PycharmProjects/phew/tests/logging_test.py", line 37, in test_truncate_after_last_newline
    self.assertEqual("", l.read(4))
AssertionError: '' != '2021'
+ 2021

======================================================================
FAIL: test_truncate_after_last_newline_no_newline_at_eof (__main__.TestUtils)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/charlie/PycharmProjects/phew/tests/logging_test.py", line 99, in test_truncate_after_last_newline_no_newline_at_eof
    self.assertEqual("", l.read(4))
AssertionError: '' != '2021'
+ 2021

======================================================================
FAIL: test_truncate_before_first_newline (__main__.TestUtils)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/charlie/PycharmProjects/phew/tests/logging_test.py", line 60, in test_truncate_before_first_newline
    self.assertEqual(size, truncated_size + 55)
AssertionError: 8724 != 7785

======================================================================
FAIL: test_truncate_between_first_and_last_newlines (__main__.TestUtils)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/charlie/PycharmProjects/phew/tests/logging_test.py", line 50, in test_truncate_between_first_and_last_newlines
    self.assertEqual(size, truncated_size + 63)
AssertionError: 4000 != 4312

======================================================================
FAIL: test_truncate_on_newline (__main__.TestUtils)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/charlie/PycharmProjects/phew/tests/logging_test.py", line 69, in test_truncate_on_newline
    self.assertEqual("2021-01-01 00:02:21", l.read(19))
AssertionError: '2021-01-01 00:02:21' != '2021-01-01 00:09:32'
- 2021-01-01 00:02:21
?                 ---
+ 2021-01-01 00:09:32
?                +++


======================================================================
FAIL: test_truncate_to_zero (__main__.TestUtils)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/charlie/PycharmProjects/phew/tests/logging_test.py", line 88, in test_truncate_to_zero
    self.assertEqual("", l.read(4))
AssertionError: '' != '2021'
+ 2021

======================================================================
FAIL: test_truncate_to_zero_no_newline_at_eof (__main__.TestUtils)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/charlie/PycharmProjects/phew/tests/logging_test.py", line 109, in test_truncate_to_zero_no_newline_at_eof
    self.assertEqual("", l.read(4))
AssertionError: '' != '2021'
+ 2021

----------------------------------------------------------------------
Ran 8 tests in 0.007s

FAILED (failures=7)

@macifell
Copy link
Copy Markdown

It's great to see a more complete solution to this!

I tried running the tests on your branch, but a few of them failed.

test_truncate_after_end_of_file (tests.logging_test.TestUtils.test_truncate_after_end_of_file) ... ok
test_truncate_after_last_newline (tests.logging_test.TestUtils.test_truncate_after_last_newline) ... ok
test_truncate_after_last_newline_no_newline_at_eof (tests.logging_test.TestUtils.test_truncate_after_last_newline_no_newline_at_eof) ... ok
test_truncate_before_first_newline (tests.logging_test.TestUtils.test_truncate_before_first_newline) ... FAIL
test_truncate_between_first_and_last_newlines (tests.logging_test.TestUtils.test_truncate_between_first_and_last_newlines) ... FAIL
test_truncate_on_newline (tests.logging_test.TestUtils.test_truncate_on_newline) ... FAIL
test_truncate_to_zero (tests.logging_test.TestUtils.test_truncate_to_zero) ... ok
test_truncate_to_zero_no_newline_at_eof (tests.logging_test.TestUtils.test_truncate_to_zero_no_newline_at_eof) ... ok

======================================================================
FAIL: test_truncate_before_first_newline (tests.logging_test.TestUtils.test_truncate_before_first_newline)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/tests/logging_test.py", line 60, in test_truncate_before_first_newline
    self.assertEqual(size, truncated_size + 55)
AssertionError: 8724 != 8675

======================================================================
FAIL: test_truncate_between_first_and_last_newlines (tests.logging_test.TestUtils.test_truncate_between_first_and_last_newlines)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/tests/logging_test.py", line 50, in test_truncate_between_first_and_last_newlines
    self.assertEqual(size, truncated_size + 63)
AssertionError: 4000 != 4031

======================================================================
FAIL: test_truncate_on_newline (tests.logging_test.TestUtils.test_truncate_on_newline)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/tests/logging_test.py", line 69, in test_truncate_on_newline
    self.assertEqual("2021-01-01 00:02:21", l.read(19))
AssertionError: '2021-01-01 00:02:21' != '2021-01-01 00:02:22'
- 2021-01-01 00:02:21
?                   ^
+ 2021-01-01 00:02:22
?                   ^


----------------------------------------------------------------------
Ran 8 tests in 0.003s

FAILED (failures=3)

@ccrighton
Copy link
Copy Markdown
Author

@macifell thanks for the review, I'll take a look.

@ccrighton
Copy link
Copy Markdown
Author

ccrighton commented Jun 27, 2024

@macifell The issue was that the tests were sensitive to the line ending. I've created a .gitattributes file to ensure that the log files have LF only. I rewrote the history of that branch.

@macifell
Copy link
Copy Markdown

macifell commented Jul 2, 2024

Thanks @ccrighton! The tests all pass for me now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants