Skip to content

Unit Tests #41

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

Closed
ajoslin opened this issue Mar 6, 2013 · 11 comments
Closed

Unit Tests #41

ajoslin opened this issue Mar 6, 2013 · 11 comments

Comments

@ajoslin
Copy link
Contributor

ajoslin commented Mar 6, 2013

I hate to be that guy, but unit tests are really really needed. I'd be willing to help with these if the APIs are final.

@timkindberg
Copy link
Contributor

I think they've started them, but perhaps more are needed. @ksperling will have a feel for what is needed here.

@ksperling
Copy link
Contributor

I had a look at doing some work on the tests today, however the support for testing promises seems rather minimal to absent in jasmine + angular-mock. I'd like to be able to make expectations of promises, but haven't had much luck so far (because core functionality like $state.transitionTo, resolve, ... are all based on promises).

This looks promising though: https://gist.github.com/groner/2187487

One thing I find odd is that angular-mocks turns $timeout and $http from real async into whats effectively queues that need to be manually flushed, but you need to know what the code under test is doing in detail to flush the correct queue, and there's no support for $q at all.

@ksperling
Copy link
Contributor

BTW, I've essentially copied the jasmine/testacular setup from the main AngularUI project; I'm not sure if there is any difference between this setup and the AngularJS e2e runner, or why one would be preferable over the other. If anybody can shed some light on these problems that would be great.

@nprbst
Copy link

nprbst commented Mar 7, 2013

Might this be helpful?

http://chaijs.com/plugins/chai-as-promised

Sent from my iPhone

On Mar 6, 2013, at 10:49 PM, Karsten Sperling notifications@github.com
wrote:

I had a look at doing some work on the tests today, however the support for
testing promises seems rather minimal to absent in jasmine + angular-mock.
I'd like to be able to make expectations of promises, but haven't had much
luck so far (because core functionality like $state.transitionTo, resolve,
... are all based on promises).

This looks promising though: https://gist.github.com/groner/2187487

One thing I find odd is that angular-mocks turns $timeout and $http from
real async into whats effectively queues that need to be manually flushed,
but you need to know what the code under test is doing in detail to flush
the correct queue, and there's no support for $q at all.


Reply to this email directly or view it on
GitHubhttps://github.com//issues/41#issuecomment-14544124
.

@ksperling
Copy link
Contributor

@nathanprobst that looks really neat... however both AngularUI and core Angular seem to be using Jasmine rather than Mocha/Chai so I think we need to stick with Jasmine if possible so that the tests don't need to be refactored / rewritten if/when this gets merged upstream.

@ksperling
Copy link
Contributor

Would anybody care to review 133e17f, particularly the testUtils resolvedValue() part?

I think it's OK, but not 100% ideal that resolvedValue() implicitly performs a digest() cycle as that effectively "moves the test along".

ksperling added a commit that referenced this issue Mar 7, 2013
Instead wrap each promise object so the resolved value or error is exposed on the promise
@ksperling
Copy link
Contributor

I think this latest attempt is pretty neat.

@timkindberg
Copy link
Contributor

I think it looks pretty great.

@ajoslin
Copy link
Contributor Author

ajoslin commented Mar 7, 2013

Looks cool! You could register jasmine expectations for these resolvedValue thingys to make things cleaner.

We did this in ui-bootstrap for toHaveClass: https://github.com/angular-ui/bootstrap/blob/master/misc/test-lib/helpers.js

You could do something like this:

beforeEach(function() {
  this.addMatchers({
    toResolveWithValue: function(value, deepCompare) {
      this.message = function() {
        return "Expected '" + angular.mock.dump(this.actual) + "' to resolve with value '" + value + "'.";
      };
      var result = resolvePromise(this.actual);
      if (result.success) {
        return deepCompare ? result.value == value : result.value === value;
      }
      return false;
    },
    toResolveWithError: function(value, deepCompare) { 
      this.message = function() {
        return "Expected '" + angular.mock.dump(this.actual) + "' to resolve with error.";
      };
      var result = resolvePromise(this.actual);
      if (result.error) {
        //if we don't give an error value to match, just pass if any error happens. 
        //similar to jasmine expect.toThrow();
        if (!angular.isDefined(value)) {
          return true;
        } else {
          return deepCompare ? result.value == value : result.value === value;
        }
      }
      return false;
    }
  });
});
expect($state.transition).toResolveWithValue($state.current);
expect(evilPromise).toResolveWithError();

@ksperling
Copy link
Contributor

Hm that would be neat, however you end up replicating the actual matching as well... Ideally you'd be able to register it as a matcher modifier, like "not", so you could say expect(xyz).eventually.toBe(...)

ksperling added a commit that referenced this issue Mar 8, 2013
Support for specifying state.params explicitly
Pass stateParams to $stateChange* events
Support preventDefault on $stateChangeStart (#14)
Also more tests (#41)
@timkindberg
Copy link
Contributor

We've got unit tests now. If there is a specific area that needs more, open a new issue.

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

4 participants