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

Coverage test: keep stdin open #37

Merged
merged 1 commit into from
Sep 25, 2020
Merged

Coverage test: keep stdin open #37

merged 1 commit into from
Sep 25, 2020

Conversation

alxiord
Copy link
Member

@alxiord alxiord commented Sep 9, 2020

Pytest closes stdin by default, so any processes spawned from it won't have it. As some tests in various rust-vmm crates might need stdin to be open, we mock it with a (currently empty) byte stream, which we can extend in the future.

@alxiord alxiord requested a review from sameo as a code owner September 9, 2020 09:50
subprocess.run(kcov_cmd, shell=True, check=True)
# Pytest closes stdin by default, but some tests might need itto be open.
# In the future, should the need arise, we can feed custom data to stdin.
subprocess.run(kcov_cmd, shell=True, check=True, input=b'')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked through the documentation, but cannot really understand what is the impact of having input=empty byte. It seems like this means that stdin will be a PIPE? Really not sure what is going on there. Could you expand on what happens (not in the code, but just in a comment here :)))

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stdin won't be a PIPE, it will be a stream composed of the bytes passed here through input=. Without specifying stdin=<something> in the subprocess.run call, stdin gets closed by default. Some tests (looking at you, vmm-reference with stdin serial) expect stdin to be open, even if there's nothing coming through. Since the behavior before was to close it, passing an empty byte stream doesn't break existing behavior while circumventing the problem.

Pytest closes stdin by default, so any processes
spawned from it won't have it. As some tests in
various rust-vmm crates might need stdin to be
open, we mock it with a (currently empty) byte
stream, which we can extend in the future.

Signed-off-by: Alexandra Iordache <aghecen@amazon.com>
@lauralt lauralt merged commit 265df53 into rust-vmm:master Sep 25, 2020
@alxiord alxiord deleted the stdin branch October 15, 2020 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants