Missing angular prefix from isDefined calls in ngMock $interval service

[mjtko]

As mentioned in line notes on 2b5ce84, the merge of #4047 introduced two plain calls to an undefined isDefined function in ngMock which should be calls to angular.isDefined instead.

2b5ce84#diff-2a255ed5e9564e25ce6eb711b604f40fR470 and 2b5ce84#diff-2a255ed5e9564e25ce6eb711b604f40fR472 are the lines in question.

I would submit a PR, but I don't want my first-time contribution to Angular to be spec-less, and I'm at a loss as to how to go about providing failing tests (or indeed, any tests!) in this particular instance. Let me know if you want me to knock up a PR anyway (though I'm sure somebody from the Angular team can handle this more rapidly than that :-)).

[petebacondarwin]

@mjtko - You are right about this, sort of. If this was in the "real" world then this would crash every time. But the reason it does not fail is that ngMock is only ever loaded up in unit tests. And in this case the unit tests are loaded up in the same global namespace as the ng source, which does indeed define isDefined() globally.

So it is not really a big problem, but to keep consistency then it ought to have angular. prefixed. Why not send in a PR anyway. We can always add a test in if it can be worked out.

[mjtko]

@petebacondarwin - Ok, I'll knock up a PR, thanks.

I'm certainly seeing this fail like this, but perhaps it's the way I have my environment configured. I don't have isDefined in my global namespace. Perhaps I'm missing something? I'll try and create a minimal example that demonstrates what I'm seeing.

[petebacondarwin]

In your tests it will fail. This is why it is wrong. In angular project
tests it doesn't because if how karma loads up the source files.
On 9 Oct 2013 15:48, "Mark J. Titorenko" notifications@github.com wrote:

@petebacondarwin https://github.com/petebacondarwin - Ok, I'll knock up
a PR, thanks.

I'm certainly seeing this fail like this, but perhaps it's the way I have
my environment configured. I don't have isDefined in my global namespace.
Perhaps I'm missing something? I'll try and create a minimal example that
demonstrates what I'm seeing.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/4334#issuecomment-25977139
.

[mjtko]

Ah! Ok, so it is expected to be a problem in the real world for me then!

Sorry, I thought you were implying that it should never be a problem - but I now understand that you meant that the problem was not (and cannot) be uncovered by the unit tests of the angular project as isDefined is globally available within those tests.

Am I on the right track now?

(PS. I'll shortly STFU and get a PR written, I just want to be sure I'm understanding correctly! 😉)

[petebacondarwin]

Right
On 9 Oct 2013 16:15, "Mark J. Titorenko" notifications@github.com wrote:

Ah! Ok, so it is expected to be a problem in the real world for me then!

Sorry, I thought you were implying that it should never be a problem - but
I now understand that you meant that the problem was not (and cannot) be
uncovered by the unit tests of the angular project as isDefined is
globally available within those tests.

Am I on the right track now?

(PS. I'll shortly STFU and get a PR written, I just want to be sure I'm
understanding correctly! [image: 😉])

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/4334#issuecomment-25978433
.

[mjtko]

Perfect, thanks for getting back to me. I'm still new to Angular, so I want to be certain that I'm not getting the concepts or my infrastructure wrong!