Skip to content
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.

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

Provide more realistic stdio wrappers #51

Open
jayvdb opened this issue Sep 7, 2019 · 8 comments
Open

Provide more realistic stdio wrappers #51

jayvdb opened this issue Sep 7, 2019 · 8 comments

Comments

@jayvdb
Copy link
Contributor

jayvdb commented Sep 7, 2019

xdoctest has the same classes as doctest for the _SpoofOut, and it doesnt implement TextIOWrapper like it is supposed to, so it doesnt have attribute buffer.

https://travis-ci.org/jayvdb/stdio-mgr/jobs/581950174

It is a very common problem, which I recently fixed in the library I am working on atm bskinn/stdio-mgr#26 and bskinn/stdio-mgr#25 may be of use , or just take a look at the latest code at https://github.com/bskinn/stdio-mgr/blob/master/src/stdio_mgr/stdio_mgr.py . The intention is that those classes can be easily re-used for misc. stdio wrapping purposes, but unfortunately it is py34+ , so this library can not easily re-use it, but we have been thinking about a py27 backport at bskinn/stdio-mgr#34

@Erotemic Erotemic added the bug label Sep 7, 2019
@Erotemic
Copy link
Owner

Erotemic commented Sep 7, 2019

I'll admit that I didn't / don't know what I'm doing when it comes to properly implementing the capture logic for the io-streams.

Is the main issue that I'm using io.StringIO instead of io.TextIOWrapper?

I'm also reminding myself that this problem will also likely have to be fixed in ubelt: https://github.com/Erotemic/ubelt/blob/master/ubelt/util_stream.py

@jayvdb
Copy link
Contributor Author

jayvdb commented Sep 7, 2019

ubelt is amazingly similar to stdio-mgr. A little more advanced in some ways, and not so advanced in others. No tests directly exercising it, but a lot of usage in the tests.

I've dropped a note about it at bskinn/stdio-mgr#59 (comment) .

It has the same problems as stdio-mgr, and so the fixes in my earlier comment are likely what are needed. Effectively, you only need to wrap the TeeStringIO in a TextIOWrapper, so that it has the same structure.

Since it seems we have active maintenance here, I would be happy to have a go at fixing it in ubelt, stringing that together with xdoctest and my use case, to ensure it all works.

@jayvdb
Copy link
Contributor Author

jayvdb commented Sep 7, 2019

Ugh, now I see mahmoud/boltons#220 .
The closest they have is https://github.com/mahmoud/boltons/blob/master/boltons/ioutils.py , which is a good foundation.

@Erotemic
Copy link
Owner

Erotemic commented Sep 9, 2019

Looking into it, I didn't realize that sys.stdout was a io.TextIOWrapper itself. So, would it be sufficient to keep TeeStringIO as a io.StringIO object and then wrap the TeeStringIO in a io.TextIOWrapper when monkey patching sys.stdout?.

Also, I'm unable to reproduce conditions where the current setup fails. Would you be able to come up with a minimal working example that reproduces this problem? I would like to add it as a test.

@jayvdb
Copy link
Contributor Author

jayvdb commented Sep 9, 2019

A doctest which contains sys.stdout.buffer should fail, and sys.stdout.buffer.buf should also work but will not.

https://github.com/timrburnham/bom_open/ is very small example of the first, with its use of buffer and detach().

So, would it be sufficient to keep TeeStringIO as a io.StringIO object and then wrap the TeeStringIO in a io.TextIOWrapper when monkey patching sys.stdout?.

That fixes the basic problem. pytest has lots of bugs in this area, but ideally xdoctest aims to at least work similarly to the pytest capturing emulation of the sys.std{out|err}. That would be a good milestone.

But "sufficient" depends on what people are doing in the doctest. Going beyond bom_open support, tests using xdoctest with console libraries like colorama and click will likely expose new problems, as they wrap/fiddle with the sys.std*.

@Erotemic
Copy link
Owner

I've put a few hours into this, and I haven't really figured out the right way to do this. I'm marking this as "help wanted", so hopefully someone with more knowledge about nuances of io will see this and be able to make a PR.

@Erotemic
Copy link
Owner

@jayvdb I think I may have fixed this in #78

I found that when I was using ipdb.launch_ipdb_on_exception or IPython.embed, while executing a doctest, the embedded shell would fail because it was looking for a fileno and buffer attribute. The patch at least fixes those issues, so I suppose the wrapper is now technically more realistic, although I'm not sure if the changes address your use case.

The relevant patched file is here if you want to take a look: https://github.com/Erotemic/xdoctest/pull/78/files#diff-78c8c1b72556c0452f0efea21af8021f

@Erotemic
Copy link
Owner

@jayvdb The new release of xdoctest 0.14.0 contains the aforementioned patch if you want to try it out. If you can verify the issue persists / is fixed that would be nice.

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

No branches or pull requests

2 participants