lathonez/clicker

Deprecate in favour of driftyco/ionic-unit-testing-example

lathonez opened this issue · 23 comments

lathonez commented

Ionic are now maintaining a repo which is pretty much the same as this - it uses @angular/cli and supports unit testing. It will support e2e soon as I understand it.

https://github.com/driftyco/ionic-unit-testing-example

The purpose of this issue is to update our codebase to use their example, check everything is working, and shut everything down!

See also ionic-team/ionic-unit-testing-example#5

masimplo commented

Just wanted to thank you personally for your effort all this time. You made our lives that much easier. Your repo was the first thing I visited every time Ionic2/karma/ng-cli/node/circleci/istanbul/whatever released an update instead of spending hours to figure out what needs tinkering.

A big thanks from me and my team!

  • 👍1
lathonez commented

@masimplo thanks for all your contributions and your kind words on the other issue. It's been great working with you.

  • 👍5
  • ❤️1
kamok commented

I looked over their repo on the official driftyco one. I see some considerably different configuration, such as the lack of a Test Utils. What would be the recommended procedure to migrating to their setup?

superkew commented

Thanks for all your work. This repo has helped me extensively!

kamok commented

@superkew I wish I can get him something nice =). This repo has been vital for the past 5 months working with Ionic and Jasmine + Karma testing.

  • 👍1
lathonez commented

I see some considerably different configuration, such as the lack of a Test Utils

Max changed the structure of the repo a lot last night, now it is very different, so it is not using @angular/cli anymore.

However - if this is the way Ionic recommend to proceed we should all follow suit and contribute over there.

Re the lack of a TestUtils - this is a subjective thing. I like TestUtils (I am testing a large project based on this code with over 450 tests, and I like consolidating the boilerplate). If you want to keep the TestUtils, I suggest keeping it yourself in a separate utilities class.

If you don't want to keep it, you can write the code in-line: https://github.com/lathonez/clicker/blob/master/src/pages/page2/page2.spec.ts#L15-L32

The idea of this is issue is that it will show how to migrate to Ionic's new setup. I have not been able to put a lot of time into it yet.

lathonez commented

3a8b610 - two tests failing to do with button clicks events. Tried various different methods. Latest is the accepted answer here which (apart from being the correct way to do these click events), seems to work the second time around: http://stackoverflow.com/questions/40093013/testing-click-event-in-angular2-testing

Once tests are passing, need to investigate the above. Then test on larger project.

lathonez commented

Added PRs for outstanding missing functionality (as above). Just need to sort those two failing tests.

lathonez commented

Since I've moved to driftyco/ionic-unit-testing-example, I'm getting two failing tests:

I've tried re-writing these in a variety of ways (the latter is the latest recommended on SO) - nothing doing.

Tonight I've realised that if I run both those spec files in isolation, the tests run fine.. which is worrying?

lathonez commented

Removing app.component.spec.ts makes everything pass.

lathonez commented

This is super weird, adding any test (even something like this):

  it('it is true', () => {
    expect(true).toBe(true);
  });

it('doesnt actually test anything', () => {
    // there is nothing here
});

into app.component.spec.ts causes those other tests to fail. Adding afterEach => fixture.destroy does nothing

lathonez commented

Very difficult thing to google for.

lathonez commented

Migration / Adoption Advice

I've updated this repo according to driftyco/ionic-unit-testing-example as an evaluation.

The following issues need resolving before I will be attempting to move our closed source project (500+ unit tests) over

  • Fix issues with Jasmine typing: PR
  • Fix tests in app.component.spec.ts or remove them: issue
  • Add support for e2e tests: PR
  • Add support for test coverage: PR
  • Add script for running within CI - this isn't a deal breaker as it's a simple addition: PR

I'm also a little concerned about community support at the moment.

The big driver for using @angular/cli was not technical - it was that the angular team are behind it, it is the recommended testing framework for angular2, and as such it has a huge community following. You can be sure someone has encountered your problem before and some smart dev at angular has fixed it. I like that moving away drops some dependencies, but I don't like the lack of support.

This leaves us in a difficult position at the moment. Personally I am not going to upgrade my closed source project (running an identical setup to clicker@2.9.1), until the above are resolved.

If at some point we need to take an upgrade and we're still stuck here, I'll probably revert clicker to using @angular/cli.

  • 👍2
masimplo commented

@lathonez I attempted to move my configuration to that of ionic-unit-testing-example and after a couple of hours I gave up as I couldn't get it to work in such a large project (1300+ tests currently) and even if I did I would miss quite a few features (e.g. coverage that pull request rating is based upon).
I am actually quite happy with my current setup and don't mind that I have to use ngCli and the support on that front is much much better as you mention.

I would argue that it might make sense you keep this repo as it was previously as an alternative to the official approach, at least until the official one is mature enough to replace this.

  • 👍2
lathonez commented

@masimplo - I hear you, lets see what happens in the next couple of weeks.

kamok commented

Hey everyone, I got an official ionic reply that sounds like there's going to be zero support on ionic-unit-testing-example. Here's the exact exchange.
Me:

Can anyone from the Ionic Team explain the level of support that would be given for https://github.com/driftyco/ionic-unit-testing-example repo? There's an active community on it, and multiple seeder test projects that are looking to migrate to it. Are we getting official support?

Ionic:

it's an example repo
once the actual testing process has been moved into app-scripts/CLI, it will no longer be needed

lathonez commented

Re the above: ionic-team/ionic-unit-testing-example#23

If this proves to be the case I'll roll everything back to 2.9.1 and then do an upgrade based on latest ionic conference and @angular/cli

lathonez commented

They got back to the above and added @liefwells as a maintainer from the community which is great.

All things considered I think @masimplo is correct above

I would argue that it might make sense you keep this repo as it was previously as an alternative to the official approach, at least until the official one is mature enough to replace this.

When I first raised this ticket, ionic-unit-testing-example was basically a clone of this, using @angular/cli for testing - so it was mature by definition. Soon after they binned it for a much lighter alternative which probably has a long way to go to offer the support needed for running against production code.

For now I will keep this open, running with @angular/cli.

If anyone feels strongly about it let me know, I'm not interested in diluting the community and will reference Ionic's example on the readme and blog.

  • 👍2
lathonez commented

Updated to Ionic 3.0.1 and Angular/cli 1.0.0

chr4ss1 commented

one thing I've noticed that this repo does not support unit testing libraries (eg without root module, my project has only feature modules). That is due angular cli not having the support for it: angular/angular-cli#1692 and by the sound of it, it's not coming anytime soon.

I've resorted to using the ionic official repo.