Make EncodedFile.write() return the return value from inner write()

[gavento]

Stream write() methods should/may return the number of characters or bytes written. Encoded file needlessly discarded the value returned from the wrapped stream write.
Fixes #6557

[nicoddemus]

Great work @gavento, thanks! See my suggestions. 👍

[blueyed]

👍 for @nicoddemus's suggestions.

Please squash into a single commit, and force-push then also.

[blueyed]

Should also have a return with writelines probably:

pytest/src/_pytest/capture.py

Lines 429 to 431 in 1ef137c

	def writelines(self, linelist):
	data = "".join(linelist)
	self.write(data)

[gavento]

Should also have a return with writelines probably:

pytest/src/_pytest/capture.py

Lines 429 to 431 in 1ef137c

	def writelines(self, linelist):
	data = "".join(linelist)
	self.write(data)

Thanks for the pointer, however .writelines in CPython returns None, so I would leave it as is. See the code at https://github.com/python/cpython/blob/master/Lib/_pyio.py#L583

[blueyed]

Thanks for the pointer, however .writelines in CPython returns None

I'd still make it into a return, in case that would ever change etc.
I think it is good practice in general, and do not think you would need a test.
Feel free to leave it out though, I might do it in a separate PR then.

[gavento]

@blueyed I had another suggestion I would make into a separate PR if you think it useful, that would resolve writelines more generally:
Make EncodedFIle inherit from io.TextIOBase, or io.IOBase directly.

• That would make it recognizable as a stream, inheriting from IOBase. (I was surprised by that recently).
• io.IOBase has the canonical writelines implementation, so any change would happen there.
• There are many more methods in io.TextIOBase but we do not need to define them (they raise unsupported by default). The only ones that would be good - and filelike objects are sort of expected to have regradless of IOBase inheritance - are flush(), writable(), and close() + closed.

[blueyed]

@gavento makes sense. I've quickly done #6566, which hopefully will fail (since it apparently does not inherit from IOBase yet).
Feel free to give it a try yourself.

[blueyed]

@gavento just for info: it redirects attribute lookup to the original buffer already: https://github.com/pytest-dev/pytest/pull/6566/files#diff-e4269384a671a462a4824e8b06d4644bR441-R442

[gavento]

@gavento just for info: it redirects attribute lookup to the original buffer already: https://github.com/pytest-dev/pytest/pull/6566/files#diff-e4269384a671a462a4824e8b06d4644bR441-R442

Right, 🤦‍♂️ I noticed that elegant piece earlier. In that case, adding inheritance from IOBase would break that and we would need to manually reimplement the relevant methods as redirects. This could be done by some meta-programming or __getattribute__ to save trivial code repetition, but that would make it even more obscure ;)

Other file-like objects already do not inherit IOBase .. but having that inheritance in pytest would be sort of nice.