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

Fix Jasmine Unit Test errors #1385

Merged
merged 5 commits into from
Dec 19, 2017
Merged

Fix Jasmine Unit Test errors #1385

merged 5 commits into from
Dec 19, 2017

Conversation

julianxhokaxhiu
Copy link
Contributor

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
New tests added? not needed
Fixed tickets
License MIT

Description

Fix a bunch of unit tests that were failing upon running npm run build. Now everything builds fine again.

--

Please, don't submit /dist files with your PR!

@julianxhokaxhiu
Copy link
Contributor Author

I had to fix also the Travis build. Not sure why, but you were still using NodeJS 0.10, while it's very safe to use the latest LTS. I hope it's fine for you.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 18c64e7 on julianxhokaxhiu:bugfix/test-cases into ** on yabwe:master**.

Copy link
Contributor

@j0k3r j0k3r left a comment

Choose a reason for hiding this comment

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

I really have no idea why we were on the default nodejs version on Travis. 😶
Thanks for fixing tests are fixing Travis too!
👍

Therefore make the container a bit bigger and make the test run properly.
If not, the 'left' property is set to auto and the test case fails.
Fix undefined is not a constructor (evaluating 'range.startContainer.getBoundingClientRect()')
It was too small for the toolbar to fit
@julianxhokaxhiu
Copy link
Contributor Author

@j0k3r Fixed conflicts, you can merge it now if you want.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 55dec60 on julianxhokaxhiu:bugfix/test-cases into ** on yabwe:master**.

@j0k3r
Copy link
Contributor

j0k3r commented Dec 19, 2017

You remove the change of nodejs version?

@julianxhokaxhiu
Copy link
Contributor Author

Yeah, I kept your choice in master, in order to avoid conflicts while merging :)

@j0k3r j0k3r merged commit c0bc028 into yabwe:master Dec 19, 2017
@julianxhokaxhiu julianxhokaxhiu deleted the bugfix/test-cases branch December 19, 2017 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants