Skip to content

TestProcess: Improve reading from a pipe with a sub-process #2043

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

Merged
merged 1 commit into from
Mar 29, 2019

Conversation

spevans
Copy link
Contributor

@spevans spevans commented Mar 25, 2019

  • TestProcess.runTask() uses a pipe to read the stdout of a subprocess
    during tests however sometimes the data read from the pipe is
    truncated causing issues with the testing of the response.

  • Use .readabilityHandler to read the data whilst the sub-process is
    running and then drain the pipe after the process has terminated.

- TestProcess.runTask() uses a pipe to read the stdout of a subprocess
  during tests however sometimes the data read from the pipe is
  truncated causing issues with the testing of the response.

- Use .readabilityHandler to read the data whilst the sub-process is
  running and then drain the pipe after the process has terminated.
@spevans
Copy link
Contributor Author

spevans commented Mar 25, 2019

@swift-ci test

1 similar comment
@spevans
Copy link
Contributor Author

spevans commented Mar 25, 2019

@swift-ci test

@millenomi
Copy link
Contributor

I'm so glad this works now.

@millenomi millenomi merged commit a0dd497 into swiftlang:master Mar 29, 2019
@spevans
Copy link
Contributor Author

spevans commented Apr 1, 2019

@millenomi One thing I found when using the new FileHandle API is that there is no throwing equivalent to .availableData, ie nothing that ends up calling _readDataOfLength(count, untilEOF: false) which is a shame. One possible fix might be to modify the signature of read(upToCount count: Int) to read(upToCount count: Int, untilEOF: Bool = true) or some such.

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.

2 participants