Message303286
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 ;-) |
|
Date |
User |
Action |
Args |
2017-09-28 22:27:35 | Cornelius Diekmann | set | recipients:
+ Cornelius Diekmann, vstinner, xdegaye, martin.panter, cstratak |
2017-09-28 22:27:35 | Cornelius Diekmann | set | messageid: <1506637655.36.0.466225441844.issue31158@psf.upfronthosting.co.za> |
2017-09-28 22:27:35 | Cornelius Diekmann | link | issue31158 messages |
2017-09-28 22:27:35 | Cornelius Diekmann | create | |
|