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

Async Test Environment API #3420

Closed
suchipi opened this issue Apr 29, 2017 · 5 comments · Fixed by #4506
Closed

Async Test Environment API #3420

suchipi opened this issue Apr 29, 2017 · 5 comments · Fixed by #4506

Comments

@suchipi
Copy link
Contributor

suchipi commented Apr 29, 2017

Do you want to request a feature or report a bug?

Feature request

What is the current behavior?

The API for test environments is written with the constraint that the test environment must be able to get its global ready synchronously (see the code here)

What is the desired behavior?

It'd be nice if the API was added to in order to allow the test environment to be constructed asynchronously.

Here's my use case: I'm trying to use NW.js to create an environment which opens an isolated browser window for each test. In NW.js, if you use nw.Window.open to open a new browser window, it is opened asynchronously. So it's not possible to open a window and set up its global for jest synchronously.

Here's the code I tried to use within the environment constructor:

nw.Window.open(config.testURL, (win) => {
  const global = this.global = win.window;
  installCommonGlobals(global, config.globals);
  this.moduleMocker = new mock(global);
  this.fakeTimers = new FakeTimers(global, this.moduleMocker, config);
});

This doesn't work because the callback argument to nw.Window.open is called asynchronously, so once jest gets to this line, it tries to install the console onto the environment's global, which isn't set up yet.

@cpojer
Copy link
Member

cpojer commented May 2, 2017

Yeah, we should do this. Make an async "setup" function in both TestEnvironment's and wait for it in the code. If you can send a PR tomorrow, we can include it into Jest 20.

@suchipi
Copy link
Contributor Author

suchipi commented May 2, 2017

Not promising I can have a PR in by tomorrow, but...

Do you want to use an async setup function instead of the current synchronous constructor, or in addition to? The former would be a breaking change. Since you are bumping the major version to 20 anyway (and this api is not commonly used), that would not be a very big deal, but I wanted to confirm.

@thymikee
Copy link
Collaborator

thymikee commented May 2, 2017

Ideally we would change it to async, in v20 or v21. If we can't get it into 20, we may release a minor version with 2 possible ways, along with proper depreciation warning.

@cpojer
Copy link
Member

cpojer commented May 2, 2017

I would keep the constructor, you can't get rid of a constructor anyway and add the setup function as an optional hook. If the function is present, call it and wait for the promise to resolve. If it isn't there, just do what we do now.

@github-actions
Copy link

This issue 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 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants