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

feat: allow for capture of stdout/stderr from worker #425

Merged
merged 3 commits into from Jan 18, 2024

Conversation

cpendery
Copy link
Contributor

This PR adds support for capturing a worker's stdout/stderr during a task and providing it as messages back from the worker. This is necessary because there is no supported way in Node.js to capture stdout/stderr from the worker's side.

Example use case: A test worker that doesn't want to display any stdout during the test execution, but instead display it in an error view if the test fails.

Closes #423

Signed-off-by: Chapman Pendery <cpendery@vt.edu>
@josdejong
Copy link
Owner

Thanks, this looks neat and well tested!

A few feedbacks:

  1. Can you describe the new options in the README.md? (I see you added them to the TypeScript types already 👌)
  2. I think it would be helpful to create a small example in the example directory demonstrating how to use these options, can you create that?
  3. I'm not sure if there are tests covering the case that emitStdout and emitStdError are false or not provided. If indeed so, is there a way to test that?
  4. A thought: is there actually a use case where you would want to configure emitStdout !== emitStdError? If not, we could unify them in a single option?

@cpendery
Copy link
Contributor Author

Thanks, this looks neat and well tested!

A few feedbacks:

  1. Can you describe the new options in the README.md? (I see you added them to the TypeScript types already 👌)
  2. I think it would be helpful to create a small example in the example directory demonstrating how to use these options, can you create that?
  3. I'm not sure if there are tests covering the case that emitStdout and emitStdError are false or not provided. If indeed so, is there a way to test that?
  4. A thought: is there actually a use case where you would want to configure emitStdout !== emitStdError? If not, we could unify them in a single option?

1/2. Sounds good, I'll add those! 👍
3. I can add some specific cases for that
4. I can't think of a use case for that, since they are already returned as separate events, so I think unifying them makes sense. I'll put them together under emitStdStreams

Signed-off-by: Chapman Pendery <cpendery@vt.edu>
Signed-off-by: Chapman Pendery <cpendery@vt.edu>
@josdejong
Copy link
Owner

Thanks Chapman, this looks good 👍!

I'll merge and publish your new feature now.

@josdejong josdejong merged commit 78fe8cc into josdejong:master Jan 18, 2024
3 checks passed
@josdejong
Copy link
Owner

Published now in v9.1.0. Thanks again!

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.

capture stdio during execution of a task
2 participants