Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Karma also needs to be told about ngResource #7575

Closed
wants to merge 1 commit into from

Conversation

jmpalacios
Copy link
Contributor

In step 11 the controllers' unit tests are adapted to work with the Phone service, which uses the ngResource module. But for this to work, Karma's configuration also needs to be told to load the angular-resource.js file through its karma.conf.js configuration, and any testing runs that may be happening at the time restarted for the change to take effect.

Though the diff from step 10 to step 11 does modify the Karma configuration, angular/angular-phonecat@step-10...step-11, the explanation says nothing about it, and the tests will all fail precisely because of a failure to load ngResource for no apparent reason until the module is explicitly made available to Karma through the steps described in this pull request.

Feel free to adapt the wording to whatever may be more appropriate, but the extra instructions added in this pull request seem to be absolutely indispensable to get all tests in the green again.

An alternative, of course, is to simply ship a karma.conf.js with angular-resource.js already loaded in an updated source package, and sidestep this whole problem that arises in step 11 from the start.

The controllers' unit tests are adapted to work with the Phone service, which uses the ngResource module. But for this to work, Karma's configuration also needs to be told to load the ```angular-resource.js``` file through its karma.conf.js configuration, and any testing runs that may be happening restarted for the change to take effect.

Though the diff from step 10 to step 11 does modify the Karma configuration, angular/angular-phonecat@step-10...step-11, the explanation says nothing about it, and the tests will all fail precisely because of a failure to load ngResource for no apparent reason, until it's explicitly made available to Karma through the steps described in this pull request.

Feel free to adapt the wording to whatever may be more appropriate, but the extra instructions added in this pull request seem to be absolutely indispensable to get all tests in the green again.

An alternative, of course, is to simply ship a ```karma.conf.js``` with ```angular-resource.js``` already loaded in an updated source package, and sidestep this whole problem that arises in step 11 from the start.
@mary-poppins
Copy link

Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.

  • Uses the issue template (#7575)

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

@mary-poppins
Copy link

I'm sorry, but I wasn't able to verify your Contributor License Agreement (CLA) signature. CLA signature is required for any code contributions to AngularJS.

Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match.

If you signed the CLA as a corporation, please let us know the company's name.

Thanks a bunch!

PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR.
PS2: If you are a Googler, please sign the CLA as well to simplify the CLA verification process.

@IgorMinar
Copy link
Contributor

similar change has already landed. thanks

@IgorMinar IgorMinar closed this Oct 11, 2014
@jmpalacios jmpalacios deleted the patch-1 branch October 11, 2014 18:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants