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

initial karma webpack #131

Closed
wants to merge 41 commits into from
Closed

initial karma webpack #131

wants to merge 41 commits into from

Conversation

cescoferraro
Copy link

karma is running with webpack. But some test dont pass.
I get Error: No provider for TestComponentBuilder!
At this moment this PR is just to share code. It's not ready at all.

@lathonez
Copy link
Owner

nice I will take a look today.

@lathonez
Copy link
Owner

I don't follow why you've added TestComponentBuilder to ClickerButton.spec.ts.

TestComponentBuilder is used in asyncCallbackFactory here. It isn't necessary anywhere else.

Have a look at this post for some background.

Also when I run npm test (ditto travis), I get lots of:

ERROR in Entry module not found: Error: Cannot resolve module 'awesome-typescript' in /home/lathonez/code/clicker

have you installed awesome-typescript globally?

Note that choosing a typescript loader is exactly the kind of thing that would potentially need to change if Ionic went another way with their Webpack build.

@cescoferraro
Copy link
Author

I was just messing with the TestComponentBuilder thing around to see where it breaks. On my machine 17/29 tests fail beacause of an issue with TestComponentBuilder not being injected to the tests.

I forgot to --save-dev the awesome-typescript. Will commit again to see what travis says with the proper dependencies. I think there are only two options of typescript loaders for webpack. The stardard one the has much less features than the awesome one. Its easy to change it later anyway.

But as far as I tested, we get into an this rxjs issue too later.

@cescoferraro
Copy link
Author

I think now you can see where both the rxjs and the TestComponentBuilder issues

@lathonez
Copy link
Owner

Looks like there's an issue with typescript:

s-panferov/awesome-typescript-loader#179

@cescoferraro
Copy link
Author

@lathonez Now there is only the TestComponentBuilder issue.

@lathonez
Copy link
Owner

@cescoferraro sorry I'm really busy with work this week. I'll try my best to take a look asap.

@lathonez
Copy link
Owner

I think the no provider for TestComponentBuilder is a symptom rather than a cause.

When I make the following changes:

diff --git a/test/diExports.ts b/test/diExports.ts
index 262aa89..fce1cff 100644
--- a/test/diExports.ts
+++ b/test/diExports.ts
@@ -8,6 +8,7 @@ import { Utils }                                      from '../app/services/util
 export { TestUtils }                                  from './testUtils';

 export let providers: Array<any> = [
+  TestComponentBuilder,
   disableDeprecatedForms(),
   provideForms(),
   Form,

Then I get the following:

PhantomJS 2.1.1 (Linux 0.0.0) ClickerButton initialises FAILED
    Error: No provider for Compiler! in app/components/clickerButton/clickerButton.spec.ts (line 6200)

@lathonez
Copy link
Owner

And adding compiler:

PhantomJS 2.1.1 (Linux 0.0.0) ClickerButton initialises FAILED
    Error: Runtime compiler is not loaded. Tried to compile ClickerButton in app/components/clickerButton/clickerButton.spec.ts (line 13030)

So yeah, I think it was/is blowing up on TestComponentBuilder because that's the first thing that gets touched.

@lathonez
Copy link
Owner

@lathonez
Copy link
Owner

yup.

So:

  • See the SO above
  • The setup relies on setBaseTestProviders being called (and setting the base test providers), hence the no provider for TestComponentBuilder. This is done in app.ts, as it should only be done once in the codebase (though there is a resetBaseTestProviders function). I'm unsure why webpack has broken this, it could be something as simple as the order of the imports / tests running?

The following diff will get you past the no provider for TestComponentBuilder error:

diff --git a/test/diExports.ts b/test/diExports.ts
index 262aa89..2b8c328 100644
--- a/test/diExports.ts
+++ b/test/diExports.ts
@@ -1,12 +1,17 @@
+import {
+  TEST_BROWSER_DYNAMIC_APPLICATION_PROVIDERS, TEST_BROWSER_DYNAMIC_PLATFORM_PROVIDERS,
+}                               from '@angular/platform-browser-dynamic/testing';
 import { provide, Type }                              from '@angular/core';
 import { ComponentFixture, TestComponentBuilder }     from '@angular/compiler/testing';
-import { inject, async }                              from '@angular/core/testing';
+import { inject, async, setBaseTestProviders }                              from '@angular/core/testing';
 import { disableDeprecatedForms, provideForms, FormControl } from '@angular/forms';
 import { App, Config, Form, NavController, Platform } from 'ionic-angular';
 import { ConfigMock, NavMock }                        from './mocks';
 import { Utils }                                      from '../app/services/utils';
 export { TestUtils }                                  from './testUtils';

+setBaseTestProviders(TEST_BROWSER_DYNAMIC_PLATFORM_PROVIDERS, TEST_BROWSER_DYNAMIC_APPLICATION_PROVIDERS);
+
 export let providers: Array<any> = [
   disableDeprecatedForms(),
   provideForms(),

@lathonez
Copy link
Owner

@cescoferraro can you rebase?

@lathonez
Copy link
Owner

From the build it looks like you've still got the same problem that I mention here.

If you apply the diff in that comment to test/diExports.ts (and revert any changes you've made with regard to TestComponentBuilder, you should get by it.

import { disableDeprecatedForms, provideForms, FormControl } from '@angular/forms';
import { App, Config, Form, NavController, Platform } from 'ionic-angular';
import { ConfigMock, NavMock } from './mocks';
import { Utils } from '../app/services/utils';
export { TestUtils } from './testUtils';

// Your patch suggest I use setBaseTestProviders here?
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lathonez your patch suggests I use setBaseTestProviders here instead of app.spec.ts.
I get this error

Failed: addProviders can't be called after the injector has been already cr
eated for this test. This is most likely because you've already used the injector t
o inject a beforeEach or the current it function.
Error: addProviders can't be called after the injector has been already cre
ated for this test. This is most likely because you've already used the injector to
inject a beforeEach or the current it function.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't get that issue, but you can always use resetBaseTestProviders
before the set.

On 26 Aug 2016 00:39, "cesco" notifications@github.com wrote:

In test/diExports.ts
#131 (comment):

import { disableDeprecatedForms, provideForms, FormControl } from '@angular/forms';
import { App, Config, Form, NavController, Platform } from 'ionic-angular';
import { ConfigMock, NavMock } from './mocks';
import { Utils } from '../app/services/utils';
export { TestUtils } from './testUtils';

+// Your patch suggest I use setBaseTestProviders here?

@lathonez https://github.com/lathonez your patch suggests I use
setBaseTestProviders here instead of app.spec.ts.
I get this error

Failed: addProviders can't be called after the injector has been already cr
eated for this test. This is most likely because you've already used the
injector t
o inject a beforeEach or the current it function.

Error: addProviders can't be called after the injector has been already cre
ated for this test. This is most likely because you've already used the
injector to
inject a beforeEach or the current it function.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/lathonez/clicker/pull/131/files/6020f8148377bc7b8877350b074679f61b73b9ab#r76255563,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AG5tSJw592b9xOQe4HWonZj4GkGTkzvZks5qjajlgaJpZM4JjxWv
.

@cescoferraro
Copy link
Author

Travis is green now. Errors vanished after I commented out clickers.spec.ts and clickerButton.spec.ts

clickers.spec.ts => this.store.let is not a function ...
clickerButton.spec.ts => addProviders can't be called after the injector .....

// testSpec.instance['clicker'].getCount = function (): number { return 10; };
// });
let beforeEachFn: Function = ((testSpec) => {
// addProviders([{ provide: TestComponentBuilder, useClass: null }]);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commenting this line make the error go away

@cescoferraro
Copy link
Author

Still got the this.store.let is not a functionon the new ngrx service test

@lathonez
Copy link
Owner

lathonez commented Aug 26, 2016

Nice! We'll definitely use this as a base whenever Ionic switch over to Webpack.

The only thing missing afaics is a coverage report. Any ideas on getting the report back?

EDIT: when this is done I'll branch off master and merge this in.

@cescoferraro
Copy link
Author

cescoferraro commented Aug 31, 2016

Hans @hansl 15:50
there’s a bug ight now between Awesome-Typescript-Loader and Typescript 2.0.2
if you create a new project you’ll get 2.0.2. npm install typescript@2.0.0 (and potentially save it in your pacakge.json) and you’ll be fine

@lathonez Angular itself is using awesome-typescript-loader. I find it hard for the ionic team to use anything else.I will push the coverage report feauture. I got it to work, but the numbers are really low. Will try to take a look again today.

@lathonez
Copy link
Owner

lathonez commented Sep 1, 2016

ionic-team/ionic-conference-app#168

We are going to leverage the Angular-CLI's set-up for karma and protractor as well.

Which would mean to end of this project altogether (as a reference point for a unit testing framework, anyway). Which is a good thing for the community.

@lathonez
Copy link
Owner

rc0 is out, they've gone with rollup not webpack.

Please reopen if I've missed something.

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

Successfully merging this pull request may close these issues.

4 participants