Cornelius Diekmann <c.diekm...@googlemail.com> added the comment:

Yes, I get the AssertionError with the latest version of PR 3802. From the high 
load avg of my system, you can see that the error occurs very rarely and that I 
need to stress my system to trigger it. With PR 3802, the error occurs way less 
frequently than without it. So we are definitely moving into the right 
direction. With PR 3808, I could not trigger any error (on my system).

Changing the conditions to "b'\n' not in s2" should work. But we actually mean 
to read a line, so this should be better reflected in the code. I prefer 
calling a readline() function directly, which is almost self-documenting code.

> Your code calls read(1) in a loop until it gets the newline character b'\n'.
> Is it better to os.read(1024) in a loop until the output string contains 
> b'\n'?
Behavior may be different if there are multiple short lines in the buffer 
(which should not be the case in the unit test, but this may be a problem if 
somebody copies the code and uses it somewhere else).
pitrou in the discussion of PR 3808 suggests the ultimate answer to the 
question: Just use an existing readline function from the library :-)
I added this to PR 3808.

Personal thought: I care about good code in the unit tests because people might 
look at this as reference how to use a module and copy&paste from it. I want 
the tests to be deterministic, which---as long as the tests pass---implies a 
stable CI ;-)

----------

_______________________________________
Python tracker <rep...@bugs.python.org>
<https://bugs.python.org/issue31158>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to