rust-cli / rexpect

.github/workflows/ci.yml
https://docs.rs/rexpect
MIT License
328 stars 56 forks source link

Remove sporadicly failing test #99

Closed matthiasbeyer closed 1 year ago

matthiasbeyer commented 1 year ago

This is obviously not how it is done. This patch will be merged to master so that we can work on master without issues, but a PR will be filed to revert this patch immediately... with a fix, if someone can come up with one.


This currently blocks the 0.6.0 release. I know that this is not how it is done, but I don't see a better way.

matthiasbeyer commented 1 year ago

bors merge=petreeftime

bors[bot] commented 1 year ago

Build failed:

petreeftime commented 1 year ago

I think the issue might be due to some shared resource between tests. All tests execute on their own thread inside a single program and their order of execution is not guaranteed, so I think order of execution of tests or some sort of race condition might play a role in the observed bug, due to some resource which is incorrectly shared between them.

matthiasbeyer commented 1 year ago

Hm... but the tests execute subprocesses and read and write their stdin/stdout... I don't see where are shared resources, tbh.

petreeftime commented 1 year ago

https://github.com/rust-cli/rexpect/blob/59213cdb11c8308daed5c2f5ecbe11db843ad71f/src/process.rs#L89

On the call to posix_openpt, I wonder if the O_NOCTTY flag might be required, the docs seem to imply it changes the master for the calling process.

petreeftime commented 1 year ago

I've tested this and it seems to pass tests on my repo.

https://github.com/petreeftime/rexpect/actions/runs/4779091507

petreeftime commented 1 year ago

Let me create a PR for the fix.

petreeftime commented 1 year ago

Attempted fix in https://github.com/rust-cli/rexpect/pull/101

matthiasbeyer commented 1 year ago

Rebased to latest master, to see what's happening.

matthiasbeyer commented 1 year ago

bors try

bors[bot] commented 1 year ago

try

Build failed: