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

Override "process.send" with a mock #5904

Merged
merged 1 commit into from
Mar 30, 2018
Merged

Override "process.send" with a mock #5904

merged 1 commit into from
Mar 30, 2018

Conversation

mjesun
Copy link
Contributor

@mjesun mjesun commented Mar 30, 2018

When creating a mock object for process, process.send is just referenced from the parent context, which means that a test has access to the real IPC channel established by jest-worker, thus breaking the sandbox.

As per the Node definition, process.send is not defined if the Node process hasn't started off a fork call; but this shouldn't be an issue, because send has always been exposed and no one complained so far 🙃

This should also fix #5891.

When creating a mock object for `process`, `process.send` is just referenced from the parent context, which means that a test has access to the real IPC channel established by `jest-worker`, thus breaking the sandbox.

As per the Node definition, `process.send` is not defined if the Node process hasn't started off a `fork` call; but this shouldn't be an issue because `send` has always been exposed and no one complained so far 🙃
Copy link
Member

@cpojer cpojer left a comment

Choose a reason for hiding this comment

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

  • Maybe test this somehow?
  • It's a company holiday today, don't work too much.

@@ -90,6 +90,7 @@ export default function() {
}

newProcess.env = createProcessEnv();
newProcess.send = () => {};
Copy link
Member

Choose a reason for hiding this comment

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

Any other functions on it while we're here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've thought about it and I don't think so. Maybe newProcess.exit, but I thougth it was better to let it exit.

Copy link
Member

@SimenB SimenB Mar 31, 2018

Choose a reason for hiding this comment

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

Might be cool to log a "process.exit() called from test" or something, potentially with a stack. Rarely on purpose, is it?

@mjesun mjesun merged commit 10b50c4 into master Mar 30, 2018
@mjesun mjesun deleted the process-send-mock branch March 30, 2018 22:05
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unexpected response from worker
4 participants