search

JanStevens / angular-growl-2

growl-like notifications for angularJS projects
MIT License
285 stars 97 forks source link

Fixes #81 replacing $timeout by $interval #85

Closed gastonelhordoy closed 8 years ago

gastonelhordoy commented 9 years ago

...and making notifications being testable by protractor

gastonelhordoy commented 9 years ago

I ran the tests in karma, tried it out manually and also executed my E2E protractor tests. Everything looks good. This will really help in making protractor tests faster by not waiting for notifications to be closed.

flippinjoe commented 9 years ago

Would you mind adding your karma tests to the project?

mikey0000 commented 9 years ago

+1 $timeout breaks protractor tests

mikey0000 commented 9 years ago

What can I do to get this merged? This is a bit of a blocker for our project E2E tests.

flippinjoe commented 9 years ago

From above

"Would you mind adding your karma tests to the project?"

This still applies

mikey0000 commented 9 years ago

@flippinjoe21 If its what it takes I'll submit a pull request with tests myself.

Swilvan commented 8 years ago

What I'm wondering about is that this shouldn't change any functionality right? So why wouldn't the normal tests succeeding be good enough?

gastonelhordoy commented 8 years ago

Sorry @flippinjoe21 for the delay in replying. So yes, I think @Swilvan is right, I didn't add any new feature, so existing test cases should be enough. How could I cover this change in any way that has not yet been covered by existing karma tests?

flippinjoe commented 8 years ago

@gastonelhordoy All I'm asking is that you add in the tests that you created. Mostly because of the lack of testing that is currently being done. I'm not asking you to add tests to verify your change. Simply add in your tests so that they will be available going forward.

Also at this point you need to merge your copy with the latest since there have been some changes made causing conflicts.

Swilvan commented 8 years ago

So I created a PR with this fix without any merge issues, would be nice to have this update since it really speeds up protractor if you use these notifications a bit.

flippinjoe commented 8 years ago

Closed in favor of https://github.com/JanStevens/angular-growl-2/pull/103