Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
lathonez opened this issue Mar 23, 2017 · 23 comments
Closed

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

lathonez opened this issue Mar 23, 2017 · 23 comments

Comments

@lathonez
Copy link
Owner

@lathonez lathonez commented Mar 23, 2017

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
Copy link

@masimplo masimplo commented Mar 23, 2017

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!

@lathonez
Copy link
Owner Author

@lathonez lathonez commented Mar 23, 2017

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

@kamok
Copy link

@kamok kamok commented Mar 23, 2017

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
Copy link

@superkew superkew commented Mar 23, 2017

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

@kamok
Copy link

@kamok kamok commented Mar 23, 2017

@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.

@lathonez
Copy link
Owner Author

@lathonez lathonez commented Mar 23, 2017

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
Copy link
Owner Author

@lathonez lathonez commented Mar 24, 2017

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
Copy link
Owner Author

@lathonez lathonez commented Mar 28, 2017

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

@lathonez
Copy link
Owner Author

@lathonez lathonez commented Mar 29, 2017

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
Copy link
Owner Author

@lathonez lathonez commented Mar 29, 2017

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

@lathonez
Copy link
Owner Author

@lathonez lathonez commented Mar 29, 2017

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
Copy link
Owner Author

@lathonez lathonez commented Mar 29, 2017

Very difficult thing to google for.

@lathonez
Copy link
Owner Author

@lathonez lathonez commented Mar 29, 2017

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.

@masimplo
Copy link

@masimplo masimplo commented Mar 29, 2017

@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.

@lathonez
Copy link
Owner Author

@lathonez lathonez commented Mar 29, 2017

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

@kamok
Copy link

@kamok kamok commented Apr 3, 2017

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
Copy link
Owner Author

@lathonez lathonez commented Apr 3, 2017

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
Copy link
Owner Author

@lathonez lathonez commented Apr 5, 2017

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.

@lathonez
Copy link
Owner Author

@lathonez lathonez commented Apr 11, 2017

Updated to Ionic 3.0.1 and Angular/cli 1.0.0

@lathonez lathonez closed this Apr 11, 2017
@ChrisEelmaa
Copy link

@ChrisEelmaa ChrisEelmaa commented Jun 4, 2017

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.