search

lathonez / clicker

Ionic 2 + @angular/cli Seed Project : Angular2 + Typescript + Karma + Protractor + Travis
http://lathonez.com/2018/ionic-2-unit-testing/
MIT License
430 stars 136 forks source link

Service & mock passed from component instead of adding in test.ts #179

Closed muhammedbasilsk closed 7 years ago

muhammedbasilsk commented 7 years ago

Hey @lathonez,

I understand all your components and pages require clickerService and hence you set the service and its mock in test.ts itself.

As your blog is used as the reference to "How to add unit testing in ionic 2", those who are new to this might get confused by seeing the custom services and mocks added in the test.ts

The changes done in this PR allow the devs to pass the provider and mock from the component rather than setting it up in the test.ts, For some larger apps, it will keep the test.ts clean as well.

I think my changes might avoid this confusion. (May be I was the only one had that confusion).

Please check this and let me you know your thoughts.

lathonez commented 7 years ago

Hey,

I'm AFK for a couple of weeks, will respond when I'm back.

Thanks

On 23 Nov 2016 18:38, "muhammed basil" notifications@github.com wrote:

Hey @lathonez https://github.com/lathonez,

I understand all your components and pages require clickerService and hence you set the service and its mock in test.ts itself.

As your blog is used as the reference to "How to add unit testing in ionic 2", those who are new to this might get confused by seeing the custom services and mocks added in the test.ts

The changes done in this PR allow the devs to pass the provider and mock from the component rather than setting it up in the test.ts, For some larger apps, it will keep the test.ts clean as well.

I think my changes might avoid this confusion. (May be I was the only one had that confusion).

Please check this and let me you know your thoughts.

You can view, comment on, or merge this pull request online at:

https://github.com/lathonez/clicker/pull/179 Commit Summary

  • Service and its mock passed from component instead of setting it in the test.ts

File Changes

Patch Links:

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/lathonez/clicker/pull/179, or mute the thread https://github.com/notifications/unsubscribe-auth/AG5tSMqkS4p7VoD2v2MXH3lgwP2Cgpd0ks5rA-2AgaJpZM4K6SoY .

lathonez commented 7 years ago

Hey,

Thanks a lot for this.

As your blog is used as the reference to "How to add unit testing in ionic 2", those who are new to this might get confused by seeing the custom services and mocks added in the test.ts

I agree it is a bit confusing - but is explained in the blog. I'd be happy for a PR to the blog to adjust the wording if necessary.

For some larger apps, it will keep the test.ts clean as well.

I have a large app (~60 spec files), the idea of test.ts is to centralise all the boilerplate so you don't need to add it in each spec file. It's much easier to change a service or provider in one file (test.ts), than it is in all of your spec files. Why does it matter if your test.ts provides stuff that isn't being used in your spec?

The changes done in this PR allow the devs to pass the provider and mock from the component rather than setting it up in the test.ts

I think this is a very good point, I'm going to modify the app so that one of the spec files has the whole setup done manually so the user can see how to do it rather than going via test.ts if necessary. I'll also update the blog section to reference that along with test.ts to hopefully make the connection clearer.

I'll reference those changes here with done.

muhammedbasilsk commented 7 years ago

Hey @lathonez,

Thanks for the reply.

For some larger apps, it will keep the test.ts clean as well.

The boiler plate approach, I really liked it. But only thing strikes me is the mocking services only for specific components. And if we add mocks like 50 in test.ts they way it is right now, it all will be available to the components which doesn't need them, right? That's what I was thinking all the time.

So I think we may provide an option for the devs make choice. either put everything in test.ts, or pass service and mock service from spec. What you think? Happy to create PR for that if you agree :-)