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

Testing fails when template contains ngModel #108

Closed
chron0 opened this issue Jun 28, 2016 · 20 comments
Closed

Testing fails when template contains ngModel #108

chron0 opened this issue Jun 28, 2016 · 20 comments

Comments

@chron0
Copy link
Contributor

chron0 commented Jun 28, 2016

Thanks for sharing and updating this seed. It was a tremendous help to get me started with testing at all (no prior knowledge). I'm not sure if I'm doing it wrong or if it's a PhantomJS issue - but as soon as I use ngModel in my template, it fails the test with:

TasksPage
    ✖ initialises
      PhantomJS 2.1.1 (Linux 0.0.0)
    Failed: EXCEPTION: Error in build/pages/tasks/tasks.html:13:17
    ORIGINAL EXCEPTION: No value accessor for ''
    ERROR CONTEXT:
    [object Object]
    invoke@/home/chrono/src/governess/governess-app/node_modules/zone.js/dist/zone.js:323:34
    run@/home/chrono/src/governess/governess-app/node_modules/zone.js/dist/zone.js:216:50
    /home/chrono/src/governess/governess-app/node_modules/zone.js/dist/zone.js:571:61
    invokeTask@/home/chrono/src/governess/governess-app/node_modules/zone.js/dist/zone.js:356:43
    runTask@/home/chrono/src/governess/governess-app/node_modules/zone.js/dist/zone.js:256:58
    drainMicroTaskQueue@/home/chrono/src/governess/governess-app/node_modules/zone.js/dist/zone.js:474:43
    invoke@/home/chrono/src/governess/governess-app/node_modules/zone.js/dist/zone.js:426:41

The test works when I use ng-model instead, but that breaks angular2/ionic2 function :/ Any ideas how to get that implemented as well?

@lathonez
Copy link
Owner

Does #100 help you?

@chron0
Copy link
Contributor Author

chron0 commented Jun 28, 2016

You mean replacing [(ngModel)]=foo with just ngModel=foo or using ngControl=foo instead?

@lathonez
Copy link
Owner

(ngControl)=foo

Seemed to work in #100

On 28 Jun 2016 21:23, "chrono" [email protected] wrote:

You mean replacing [(ngModel)]=foo with just ngModel=foo or using
ngContro=fool instead?


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#108 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AG5tSGl10hHiJRMVRw_ETPYUmtY5dcToks5qQQRPgaJpZM4I_6Sk
.

@chron0
Copy link
Contributor Author

chron0 commented Jun 28, 2016

Ouh... beta10 was just released. I'll try to merge your latest changes and the new DI boilerplate (however that may work) into my codebase and will try with (ngControl)=foo even though it's not technically a form:

https://github.com/apollo-ng/governess/blob/master/governess-app/app/pages/tasks/tasks.html

@lathonez
Copy link
Owner

Agreed it looks wrong. I just followed that stack overflow through which
removed the error on #100. Might try implementing something similar on
ClickerForm to replicate.

Let me know what you end up going with.

It's my Ionic day tomorrow so I'll probably upgrade to beta10.

On 28 Jun 2016 21:46, "chrono" [email protected] wrote:

Ouh... beta10 was just released. I'll try to merge your latest changes and
the new DI boilerplate (however that may work) into my codebase and will
try with (ngControl)=foo even though it's not technically a form:

https://github.com/apollo-ng/governess/blob/master/governess-app/app/pages/tasks/tasks.html


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#108 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AG5tSP0c1nUOOR1-PelrfbYGNRHHI80jks5qQQl9gaJpZM4I_6Sk
.

@chron0
Copy link
Contributor Author

chron0 commented Jun 29, 2016

I've also upgraded to beta10 and will merge your changes tonight and then test again.

@dorontal
Copy link
Contributor

dorontal commented Jul 1, 2016

To replicate, you can add anything with an [(ngModel)]="...", e.g. just add anywhere to the clicker app the following line (in one of the html templates):

<ion-input type="text" [(ngModel)]="question" placeholder="Ask a question..."></ion-input>

You'll get the error when initializing the template's component during the test, even though the [(ngModel)] works as expected at run time.

@lathonez
Copy link
Owner

lathonez commented Jul 1, 2016

@dorontal - aha, you're here too! Thanks. Might get time to take a look this weekend.

If you happen to get a response from your post to Ionic let us know

@dorontal
Copy link
Contributor

dorontal commented Jul 1, 2016

Definitely, sir! Thanks for saving me a great deal of work, too!

lathonez added a commit that referenced this issue Jul 2, 2016
@lathonez
Copy link
Owner

lathonez commented Jul 2, 2016

see 673622d,

tests still running without the no value accessor error.

@dorontal
Copy link
Contributor

dorontal commented Jul 2, 2016

So I was wrong about <ion-input> reproducing the problem - please accept my apologies for jumping to conclusions on that one, it's because I had issues with <ion-input> in my own project, but when I tried to add an <ion-input> line (with an [(ngModel)] property) it all worked - the tests pass just fine.

However, when I add <ion-range> (with an [(ngModel)] property) - the error does reproduce.

I tried this: (1) added an <ion-range> line to page2.html

<ion-content padding class="message bye-ionic">
  <h2>Bye!</h2>
  <ion-range [(ngModel)]="value"></ion-range>
</ion-content>

(2) added the value member variable to page2.ts and initialized it

export class Page2 {
  private value: number;
  constructor() {
    this.value = 1;
    // no-op
  }

And here's the report

FAILED TESTS:
  Page2
    ✖ initialises
      PhantomJS 2.1.1 (Linux 0.0.0)
    Failed: EXCEPTION: Error in build/pages/page2/page2.html:11:13
    ORIGINAL EXCEPTION: No value accessor for ''
    ERROR CONTEXT:
    [object Object]

The <ion-range> is a brand new directive that just showed up in ionic2 beta 10. I have a feeling this will be fixed in the near future by the Ionic team.

@lathonez
Copy link
Owner

lathonez commented Jul 2, 2016

No worries.

If you still have that repro lying around a PR would be good.

Note to self to revert the changes from yesterday.

On 3 Jul 2016 01:52, "dorontal" [email protected] wrote:

So I was wrong about reproducing the problem - please accept
my apologies for jumping to conclusions on that one, it's because I had
issues with in my own project, but when I tried to add an
line (with an [(ngModel)] property) it all worked - the tests
pass just fine.

However, when I add (with an [(ngModel)] property) - the
error does reproduce.

I tried this: (1) added an line to page2.html

Bye!

(2) added the value member variable to page2.ts and initialized it

export class Page2 {
private value: number;
constructor() {
this.value = 1;
// no-op
}

And here's the report

FAILED TESTS:
Page2
✖ initialises
PhantomJS 2.1.1 (Linux 0.0.0)
Failed: EXCEPTION: Error in build/pages/page2/page2.html:11:13
ORIGINAL EXCEPTION: No value accessor for ''
ERROR CONTEXT:
[object Object]


The is a brand new directive that just showed up in ionic2
beta 10. I have a feeling this will be fixed in the near future by the
Ionic team.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#108 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AG5tSFqnwr-JcWDQEwUxmqciFAHFAPtrks5qRolQgaJpZM4I_6Sk
.

@dorontal
Copy link
Contributor

dorontal commented Jul 3, 2016

The pull request reproducing the issue is here.

It fails Travis of course (nice to see it at work) because this pull request is all about reproducing the test failure.

This merge link - Commit d70fe63 - shows the diff.

This Travis report link shows test output and has the No value for acessor '' error.

@lathonez
Copy link
Owner

lathonez commented Jul 3, 2016

@dorontal thanks a lot for the PR, looking now

@lathonez
Copy link
Owner

lathonez commented Jul 3, 2016

Interesting that it repros at page level but not component level. Particularly as @page no longer exists and they are both basically ng2 components.

@lathonez
Copy link
Owner

lathonez commented Jul 3, 2016

Just went though what's different with your example in page.ts and my example from earlier in clickerForm.ts.

Seems you need to add Ionic's TextInput directive to the component. The following diff runs fine for me for ion-input (it does not for ion-range).

x220:~/code/clicker(dorontal-master)$ git diff
diff --git a/app/pages/page2/page2.ts b/app/pages/page2/page2.ts
index 9e0662b..088c4f8 100644
--- a/app/pages/page2/page2.ts
+++ b/app/pages/page2/page2.ts
@@ -1,9 +1,11 @@
 'use strict';

 import {Component} from '@angular/core';
+import {TextInput} from 'ionic-angular';

 @Component({
   templateUrl: 'build/pages/page2/page2.html',
+  directives: [TextInput],
 })
 export class Page2 {
   // NOTE: reproducing clicker issue 108: 'value' should be a string

@lathonez
Copy link
Owner

lathonez commented Jul 3, 2016

For <ion-range> you need to add Ionic's Range directive.

lathonez added a commit that referenced this issue Jul 3, 2016
@lathonez
Copy link
Owner

lathonez commented Jul 3, 2016

I've made those changes to your PR and merged it as a demo

5d89589

Build is passing on travis

@dorontal
Copy link
Contributor

dorontal commented Jul 3, 2016

Yes. Thanks so much! I added the directives: [TextInput] or directives:[Range] line to the component's config and the error disappeared. I had originally tried to include those as providers in the test spec file and of course that did not work. Including them as directives solves the problem, i.e. we no longer get the "No value accessor for '' " error - thanks a lot for figuring this out!

@lathonez lathonez closed this as completed Jul 4, 2016
@lathonez
Copy link
Owner

#191

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

No branches or pull requests

3 participants