test($parse): add tests for watching one-time array/object literals #16477
Conversation
| return input; | ||
| })); | ||
|
|
||
| scope.$watch('[(a | foo:b:1), undefined]'); |
There was a problem hiding this comment.
What is the point of including the params b and 1 here?
Should you not also be checking what happens if b change?
There was a problem hiding this comment.
Good question... I have no idea why I added those. I guess I'll just delete them, or if I think of a reason maybe they deserve a separate test.
There was a problem hiding this comment.
Hmm... I guess I added it because the tests above and below the new tests have it as well (which I wrote some time ago)!
I think the original reason was to ensure that constant/variables as filter arguments do not effect what is being tested. While it doesn't truely test that because it never assigns a value to the b variable, I think I'd vote to leave it anyway, partially just to be consistent with the other tests around it... WDYT?
There was a problem hiding this comment.
I am indifferent, it just stood out to me as something unexplained.
|
|
||
| scope.a++; | ||
| scope.$digest(); | ||
| expect(filterCalls).toEqual([0, 1]); |
There was a problem hiding this comment.
This seems unexpected. If this is a one-time binding, shouldn't we be ignoring the filter once the binding has a value?
There was a problem hiding this comment.
Or does the undefined in the second element of the array prevent the one time binding from stabilizing?
There was a problem hiding this comment.
Exactly! For ::[ ] and ::{ }, all values must be defined before unwatching the whole expression.
jbedard
force-pushed
the
parse-onetime-literal-tests
branch
from
14903cf to
aca25d3
Compare
|
I think all the tests passed despite the failed build...? |
jbedard
force-pushed
the
parse-onetime-literal-tests
branch
from
aca25d3 to
30c836e
Compare
These are some examples of cases which 189461f enabled. All the one-time tests failed before that commit.
It's possible that these overlap with these tests a bit (the non-deep ones), but I think at least the
should ignore changes within nested objectstests are a good addition.