Skip to content
This repository has been archived by the owner on Apr 25, 2018. It is now read-only.

Fix mountable test async failures #106

Merged

Conversation

brandonpayton
Copy link
Contributor

If the "does not emit" test cases for mountable fail, they fail after the test function returns, resulting in no stack trace being reported. I stumbled on this while trying to quickly break a test to verify the karma sourcemaps PR was working as expected.

I updated the broken cases to use done. I know some don't like using setTimeout, but AFAIK, it is the most reasonable way to schedule verification after the possible event is dispatched.

I also updated a couple of test labels for unmount to use "unmount" instead of "mount".

const node = document.createElement(`div`)

// ensure that callback does not get called
const callback = () => assert.notOk(true)
const callback = sinon.spy()
Copy link
Owner

Choose a reason for hiding this comment

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

Nice catch! Though you don't need sinon here. If you use done as the callback, the test will fail if the callback is not called.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! The trick is we don't want the callback to be called, but AFAICT, mocha expects done to be called with or without an error. I initially implemented as const callback = done(new Error('...')) but was seeing errors due to done not being called.

Copy link
Owner

Choose a reason for hiding this comment

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

Derp... yup. Not thinking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ha, yep. I did the same.

@garbles garbles merged commit bfc7d52 into garbles:master May 10, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants