test(integration): verify that duplicated directives are not firing#2568
test(integration): verify that duplicated directives are not firing#2568pkozlowski-opensource wants to merge 1 commit intoangular:masterfrom
Conversation
pkozlowski-opensource
force-pushed
the
duplicate_directives
branch
from
eea3d20 to
a56d540
Compare
|
Please rebase |
tbosch
added
pr_state: LGTM
action: merge
pkozlowski-opensource
force-pushed
the
duplicate_directives
branch
from
a56d540 to
4b6800a
Compare
|
@tbosch I've rebased this one but now, after the rebase, it looks like the tested use-case doesn't work any more - that is - the test discovers a real bug! I will take a look at the underlying cause. |
|
@pkozlowski-opensource Could you investigate a little more why this is a problem? |
tbosch
added
action: cleanup
|
@tbosch I'm afraid I will need some input from you on this one, see #2756 (comment) Will ping you later today. |
pkozlowski-opensource
force-pushed
the
duplicate_directives
branch
from
4b6800a to
26ff386
Compare
|
@tbosch rebased and fixed code to remove duplicate directives as discussed yesterday. We are applying very similar logic to the one in the injector PTAL BTW, build is failing due to the NPM install issues, nothing to do with this PR... |
pkozlowski-opensource
removed
the
action: cleanup
There was a problem hiding this comment.
Could you add one more test (just for a future reader) that states: 'It should use the last directive binding per directive'?
tbosch
added
the
action: cleanup
pkozlowski-opensource
force-pushed
the
duplicate_directives
branch
from
26ff386 to
96cf845
Compare
|
@tbosch added a test. Is there anything else I should add or is this one good to go? |
pkozlowski-opensource
removed
the
action: cleanup
pkozlowski-opensource
force-pushed
the
duplicate_directives
branch
from
96cf845 to
7cc0fed
Compare
|
Please merge! |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
3 participants
I wasn't sure if we've got some mechanism in place to avoiding matching all the directives specified in the
@Viewannotation, so add a test. We might easily get into duplicate directives situation, as soon as we start to includecoreDirectivesin the@Viewby default.Not sure if this test is mandatory, but I couldn't find the place where we filter out duplicates....