-
Notifications
You must be signed in to change notification settings - Fork 22
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
Run tests on windows and mac #108
base: main
Are you sure you want to change the base?
Conversation
a895c79
to
a4b5433
Compare
a4b5433
to
27d436b
Compare
ce574f4
to
379ffa7
Compare
06f3ef4
to
54ca80c
Compare
54ca80c
to
a773bc5
Compare
ed6c4f7
to
677e6c5
Compare
677e6c5
to
05f1325
Compare
ccf0f60
to
346e22b
Compare
346e22b
to
f08eabd
Compare
fds/utils.py
Outdated
stdout, stderr = output.communicate() | ||
sys.stdout.write(convert_bytes_to_string(stdout)) | ||
sys.stderr.write(convert_bytes_to_string(stderr)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem here is that the streams won't be outputted in correct order.
If there's an error written to stderr
in the middle of output, it will still appear in the end in this way, and cause confusion.
Why not pass a splitting stream to stdout
and stderr
, which both stores the outputs in byte buffers, and writes them to sys.stdout
and sys.stderr
?
Example: https://stackoverflow.com/a/616686
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah, I forgot to do this part, We originally were doing it, I changed it so that it works in windows too
6edbce1
to
828cc3a
Compare
83a0de5
to
96a55fd
Compare
No description provided.