test($rootScope): add $watch/$watchCollection tests #15949
Conversation
test/ng/rootScopeSpec.js
Outdated
| @@ -212,10 +212,52 @@ describe('Scope', function() { | |||
| expect($rootScope.$$watchersCount).toBe(2); | |||
| })); | |||
|
|
|||
| it('should not keep constant literals on the watch queue', inject(function($rootScope) { | |||
| it('should remove $watch of constant literals after initial digest', inject(function($rootScope) { | |||
There was a problem hiding this comment.
Wrong indentation (here and below).
There was a problem hiding this comment.
I don't understand, isn't it the same as before and the same as all the others around it?
There was a problem hiding this comment.
Sorry, it turned out I had a Chrome extension that was messing the GitHub diffs. 😞
Indentation is fine.
test/ng/rootScopeSpec.js
Outdated
| expect($rootScope.$$watchers.length).toEqual(0); | ||
| })); | ||
|
|
||
| it('should remove $watchCollection of constant literals after initial digest', inject(function($rootScope) { |
There was a problem hiding this comment.
There is a $watchCollection describe in this file, I think the watchCollection specs should go there (also for all the other watchCollection tests you've added)
There was a problem hiding this comment.
That describe block has a beforeEach that does $watchCollection("var", ...) so I converted it to describe("$watchCollection var", ...).
Maybe I should add a parent describe("$watchCollection", ...) that wraps the var/literal/constant describes and can also contain this test?
I'll do that now...
There was a problem hiding this comment.
Actually this specific $watchCollection test is beside the equivalent $watch test.
What's better, putting similar watch/watchCollection beside each other or separating them into watch/watchCollection groups? I feel like this specific test is better grouped along with other constant related tests instead of with $watchCollection...
There was a problem hiding this comment.
Ok, how about wrapping the constant related watch and watchCollection tests with another describe? Btw, what about watchGroup :D
There was a problem hiding this comment.
I've wrapped the constant-cleanup and onetime-cleanup tests into describe blocks. Also added some watchGroup tests.
|
I've wrapped all the @Narretz can you clarify your comment? I'd like to get this merged so I can add more... 😃 |
|
There are some lint errors, otherwise LGTM |
|
Fixed the lint issues. I'll squash and merge later today if no one else gets to it. |
These are various cases that I felt were missing tests, mainly around constant/literal/one-time $watch/$watchCollection.