-
Notifications
You must be signed in to change notification settings - Fork 47
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
closes #95 #162
base: master
Are you sure you want to change the base?
closes #95 #162
Conversation
ea1f88e
to
7d584f4
Compare
@ankit-m - what is this check that is failing? |
@rohankatyal29 can you check it on your local machine? |
The TravisCI log shows that PhantomJS did not load. My guess is that mutation observer is causing it to fail. Is Mutation Observer supported in Chrome only? The test environment uses PhantomJS as it is headless. |
@rohankatyal29 I don't think this will be a good idea. Because if we merge this, all Travis CI checks will fail from now on, defeating its purpose. Would it be possible to find a workaround? Karma does support Chrome and Chrome Canary but with the downside that every test will open a window of Chrome. But I don't know if it will work on Travis (as it is only terminal). As an alternative, maybe we can delay this change until headless Chrome is released. It is supposed to be out soon. https://news.ycombinator.com/item?id=11839303 |
I think any workaround would have to be found in the way we do the tests. We shouldn't hack our solutions just because they are not testable with current testing technology. Testing is very important, but it is secondary to the main goal.
That would be inconvenient to maintain. |
#95