Capture: avoid using SysCapture with FDCapture #1585
Comments
|
i have to admit i am tempted to close this as offensively unactionable also there is no need to degrade into non-suspendable capture, since now if you take a deeper look into the FDCapture, which is the default, on every os that has it does simply use i suspect what you really want is not a raw or a FD capture, but a PTY Capture that has controlling fd and a controlled fd, the output/input is bidirectional between those - and that mechanism needs a background worker process/thread to transfer the data |
|
Why does FDCapture also use SysCapture? The documentation presents them as alternatives. It's the messing with sys.stdin and sys.stderr that causes Fabric to balk. WIth FDCapture, is sys.stdout.fileno() == 1 when capture is enabled? I need capture for |
|
BTW, I currently use Nose and its capture works fine with Fabric but it is broken in other ways that make me want to switch to PyTest. |
|
i see what you mean now, so what you want is a fd --only capture or a fileno attribute on stdin/out |
|
I guess so. But my question's weren't rhetorical. I really don't know the answer to them. And here's another one: Why is SysCapture even needed? If you replace file descriptors 0, 1 and 2 everything should just work. It is the commonly accepted way to redirect the standard streams and if you redirect them that way, no other means are needed. |
|
there is a legacy reason spanning back to about pytest 1.x i think |
|
i found a different interaction bug plain pytest has correct filenos pytest-xdist uses stdin/out to do process control the interaction between stdcapture and fdcapture is a bit unwanted and impossible to fix off-hand without rewriting xdist without execnet |
|
Just FYI, I'm not using xdist right now. And with |
|
then there is another bug (i just made some testcases to verify that there is a fileno please show the full traceback |
|
|
now that's way more comprehensible we dont have support for stdin capture using a fd, such tests must use pexpect or own pty control feel free to do a custom monkeypatch there also please next time do a humane bugreport and include a traceback instead of claiming everything is broken following up initially with unrelated miss-leading examples that just cost me the time to finish my marker refactor today for basically no reason and i am pretty unhappy about that |
|
Sorry, I glanced over the fact that its complaining about stdin. How very stupid of me. I still don't get why PyTest can't just replace 0, 1, 2 and leave sys.stdin, sys.stdout and sys.stderr as is. How can I prevent PyTest from replacing sys.stdin? |
|
for now i would suggest to open a pty and monkeypatching stdin.fileno() as a hack also in case of xdist, fd 0 and 1 are preoccupied and cannot be used due to the xdist protocol exchange fabric has some implementation details i disagree with (i could rant, but that wont fix any problem) it might be helpful to create a pytest plugin that monkey-patches the test capture state so that stdin is always a file (i can guide on this tomorrow and until the sprint, but i wont find the timeto tackle this before the sprint) |
Sorry for the broken release after disabling tests Fighting with pytest-dev/pytest#1585 Re-enabled tests again But cli2.autotest inside pytest cannot execute subprocess properly So we now count on executing the tests through ./sh.yml test To cover the code that actually executes scripts
Sorry for the broken release after disabling tests Fighting with pytest-dev/pytest#1585 Re-enabled tests again But cli2.autotest inside pytest cannot execute subprocess properly So we now count on executing the tests through ./sh.yml test To cover the code that actually executes scripts
blueyed
changed the title
|
Some good info in here, re-opening for now. |
|
@blueyed for tackling the actual info effectively i propose cross-linking from a new more to the point issue in near future (and closing this one once that distilled issue is created) |
|
ok, will try to keep it alive through my inbox. |
|
I also had a hard time understanding why Up to now, I found two reasons:
I still intend to try it -- will edit if I find more reasons 🙂 |
Sorry for the blunt title, but I think the capture manager is implemented badly. The proper way to redirect one of the standard file handles, say stdout, is to
os.close(sys.stdout.fileno())and then immediately callos.open()oros.dup()to open a different file, but using the same file handle. The key is that on any modern Unix, open() and dup() are guaranteed to return the index of lowest available file descriptor table entry. If you os.close(1) and then immediately call open() or dup() after that, they are guaranteed to return 1. This is how Unix shells have implemented the redirection operators>,<and|since the 1970s. The beauty of this approach is that you won't have to touchsys.stdinat all because it'sfileno()will remain the same. It is completely transparent to the higher layers.The reason I think this should be fixed is that some important software (Fabric, for example) expects
sys.stdoutto be backed by a real file handle. As PyTest's capture is currently implemented, I can only run tests involving Fabric with--capture=no. But I need to run tests in parallel and with--junitxml, so disabling capture is not really an option.I looked at capture.py and I am a a bit overwhelmed by its complexity right now, otherwise I would submit a PR. I think most of the complexity stems from the ability to temporarily suspend the capturing, which admittedly would be hard to do with the close()-then-open() approach. Maybe this technique can be implemented as
--capture=rawand it would simply not support the temporary suspension of capture.The text was updated successfully, but these errors were encountered: