-
Notifications
You must be signed in to change notification settings - Fork 5
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
#157 adds warning when global fixture is called #165
Conversation
fixture.js
Outdated
// window.fixture = fixture; | ||
|
||
window.fixture = function(){ | ||
debugger |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You forget debugger
statement.
test/fixture_test.js
Outdated
|
||
|
||
testHelpers.dev.devOnlyTest("window.fixture warns when called", function() { | ||
debugger |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another debugger
statement here.
fixture.js
Outdated
@@ -67,7 +68,14 @@ canReflect.assignMap(fixture, { | |||
}); | |||
|
|||
if(typeof window !== "undefined" && typeof require.resolve !== "function") { | |||
window.fixture = fixture; | |||
// window.fixture = fixture; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd remove this comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this, I added some comments.
- Removed debugger statements - Swapped fixture to come before steal clone test #165 (review)
For issue #157 adds warning for when global window fixture. But current test not currently running likely due to some other test failing. The test currently added works in isolation but when ran amongst other test it fails.