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

test: force context allocation in test module #18312

Closed
wants to merge 2 commits into from
Closed

test: force context allocation in test module #18312

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
62db5c2
test: force context allocation in test module
hashseed Jan 23, 2018
b5cb151
Address comment.
hashseed Jan 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
test: force context allocation in test module 
V8's behavior changed in c3bd741efd. Top-level variables
in a module are no longer context-allocated by default.
  • Loading branch information
@hashseed
hashseed committed Jan 23, 2018
commit 62db5c2f263c047215943307894ed4805fb22a7c
4 changes: 3 additions & 1 deletion test/fixtures/es-modules/loop.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,6 @@ while (t > 0) {
console.log(`Outputed message #${k++}`);
}
}
process.exit(55);
process.exit(55);

(function force_context_allocation() { return t + k; })
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a comment explaining why this is necessary. I worry that with the rate of code changes, the git commit message may get lost.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.