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

test: load common.js to test for global leaks #3095

Closed
wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Sep 27, 2015

common.js contains code that checks for variables leaking into the
global namespace. Load common.js in all tests that do not
intentionally leak variables.

common.js contains code that checks for variables leaking into the
global namespace. Load common.js in all tests that do not
intentionally leak variables.
@mscdex mscdex added the test Issues and PRs related to the tests. label Sep 27, 2015
@Trott
Copy link
Member Author

Trott commented Sep 27, 2015

@brendanashworth
Copy link
Contributor

It'd be cool if we could get a linter rule for this, that way reviewers can't miss it on new tests.

@Fishrock123
Copy link
Contributor

+1 linter rule.

@Fishrock123
Copy link
Contributor

Also lgtm

@thefourtheye
Copy link
Contributor

LGTM

@Trott
Copy link
Member Author

Trott commented Oct 2, 2015

I've written an eslint rule for this and will submit it with a subsequent pull request. Thanks for the suggestion!

@Trott
Copy link
Member Author

Trott commented Oct 2, 2015

Landed in 02fe821

@Trott Trott closed this Oct 2, 2015
Trott added a commit that referenced this pull request Oct 2, 2015
common.js contains code that checks for variables leaking into the
global namespace. Load common.js in all tests that do not
intentionally leak variables.

PR-URL: #3095
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Trott added a commit that referenced this pull request Oct 2, 2015
common.js contains code that checks for variables leaking into the
global namespace. Load common.js in all tests that do not
intentionally leak variables.

PR-URL: #3095
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
@rvagg rvagg mentioned this pull request Oct 3, 2015
@Trott Trott deleted the common branch January 13, 2022 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants