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.

Sign up for GitHub

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

Jump to bottom

Regression in Jest with jsdom #10492

Closed
cpojer opened this issue Dec 28, 2016 · 1 comment
Closed

Regression in Jest with jsdom #10492

cpojer opened this issue Dec 28, 2016 · 1 comment
Labels
console Issues and PRs related to the console subsystem. vm Issues and PRs related to the vm subsystem.

Comments

@cpojer
Copy link

cpojer commented Dec 28, 2016

  • Version: 7.3.0
  • Platform: mac os

Repro:

Two tests will fail on 7.3.0 that did not fail using 7.2.0. The concrete bug is when using --env=jsdom with Jest, console.log calls from a jsdom's vm context will not be printed any more for some reason. There is a PR in Jest to fix this: jestjs/jest#2457 but I'd like to first understand what regression in node caused this and whether this is a bug in node or an expected change.

Failed tests that expect "console.log" output to be present with the jsdom environment:
screen shot 2016-12-29 at 12 59 28 am
@cpojer cpojer mentioned this issue Dec 28, 2016
Closed
@italoacasas italoacasas added the console Issues and PRs related to the console subsystem. label Dec 28, 2016
@MylesBorins
Copy link
Contributor

MylesBorins commented Dec 28, 2016 •
edited
Loading

I'm bisecting to figure out what is going on here. Will post updates in the comment as edits

edit 1:

Looks like the commit that broke everything was 524f693

the original PR: #10227

/cc @bnoordhuis

@MylesBorins MylesBorins mentioned this issue Dec 28, 2016
Merged
@addaleax addaleax added the vm Issues and PRs related to the vm subsystem. label Dec 28, 2016
@kenany kenany mentioned this issue Jan 6, 2017
Closed
@gaearon gaearon mentioned this issue Jan 10, 2017
Closed
@addaleax addaleax mentioned this issue Jan 12, 2017
Closed
@domenic domenic mentioned this issue Jan 14, 2017
Closed
addaleax added a commit to addaleax/node that referenced this issue Jan 20, 2017
@addaleax
Revert "src: don't overwrite non-writable vm globals"
179ea5e 
This reverts commit 524f693.

Fixes: nodejs#10806
Fixes: nodejs#10492
Ref: nodejs#10227
@addaleax addaleax mentioned this issue Jan 20, 2017
Closed
targos pushed a commit that referenced this issue Jan 28, 2017
@addaleax @targos
Revert "src: don't overwrite non-writable vm globals"
69d1de1 
This reverts commit 524f693.

Fixes: #10806
Fixes: #10492
Ref: #10227
PR-URL: #10920
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 30, 2017
@addaleax @italoacasas
Revert "src: don't overwrite non-writable vm globals"
8a6367c 
This reverts commit 524f693.

Fixes: nodejs#10806
Fixes: nodejs#10492
Ref: nodejs#10227
PR-URL: nodejs#10920
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 30, 2017
@addaleax @italoacasas
Revert "src: don't overwrite non-writable vm globals"
7ff3078 
This reverts commit 524f693.

Fixes: nodejs#10806
Fixes: nodejs#10492
Ref: nodejs#10227
PR-URL: nodejs#10920
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@alexjoverm alexjoverm mentioned this issue Feb 6, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
console Issues and PRs related to the console subsystem. vm Issues and PRs related to the vm subsystem.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@cpojer @MylesBorins @addaleax @italoacasas