Skip to content
This repository was archived by the owner on Sep 5, 2024. It is now read-only.

test(mdSelect): fix tests on Angular master (v1.5.8-build.4934) #9014

Closed

Conversation

gkalpak
Copy link
Member

@gkalpak gkalpak commented Jul 12, 2016

No description provided.

@gkalpak gkalpak force-pushed the fix-mdSelect-tests-with-snapshot branch from 53f7c60 to da99202 Compare July 13, 2016 08:36
@gkalpak gkalpak changed the title tests(mdSelect): fix tests on Angular master (v1.5.8-build.4934) test(mdSelect): fix tests on Angular master (v1.5.8-build.4934) Jul 13, 2016
@ThomasBurleson
Copy link
Contributor

@gkalpak - thx
@topherfangio - is this a better fix than #9012 ?

@gkalpak
Copy link
Member Author

gkalpak commented Jul 13, 2016

@topherfangio, we were obviously working on this simultaneously 😄

So, this is essentially very similar to #9012. The main difference is that I split the test for different empty mdOptions into separate tests. This is a good idea anyway imo, because you are using the same scope ($rootScope) in all cases, which could result into one test's state affecting the subsequent tests.
(This one also avoids some unnecessary calls, but this is insignificant in the context of tests).

@topherfangio
Copy link
Contributor

That's not a bad idea. I think a merging of the two would be good? I like explicitly calling closeSelect() so that it's apparent to the user what we expect to happen.

@ThomasBurleson
Copy link
Contributor

@topherfangio - can you merge this with latest from master (which already has 9012 merged).

@topherfangio
Copy link
Contributor

@ThomasBurleson Will do; I'll shoot over a new PR in a bit.

@gkalpak
Copy link
Member Author

gkalpak commented Jul 15, 2016

Superseeded by #9041.

@gkalpak gkalpak closed this Jul 15, 2016
@gkalpak gkalpak deleted the fix-mdSelect-tests-with-snapshot branch July 15, 2016 20:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants