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

chore(ngMock): add they helper for testing multiple specs #10864

Closed
wants to merge 1 commit into from

Conversation

matsko
Copy link
Contributor

@matsko matsko commented Jan 25, 2015

No description provided.

@matsko
Copy link
Contributor Author

matsko commented Jan 25, 2015

This doesn't work yet with inject when additional modules are used and new services are injected in.

@matsko matsko closed this Jan 25, 2015
@matsko matsko reopened this Jan 26, 2015
@matsko
Copy link
Contributor Author

matsko commented Jan 26, 2015

Nevermind. This stuff works great. Please merge.

@petebacondarwin
Copy link
Member

I believe that Jasmine sets this when calling the spec function so you should too to be consistent

break;
default:
method = it;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

How about replacing this switch with something like:

var methods = {focus: iit, skip: xit, default: it};
var method = methods[specialState || 'default'];

Copy link
Member

Choose a reason for hiding this comment

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

+1

@lgalfaso
Copy link
Contributor

I think this will make it harder to move to Jasmine 2.x, as in Jasmine 2.x, the function takes an optional done parameter

@caitp
Copy link
Contributor

caitp commented Jan 27, 2015

we can handle the done parameter in the wrapper --- but who cares about jasmine 2.x anyways?

@caitp
Copy link
Contributor

caitp commented Jan 27, 2015

also note: commit message is wrong, this is not an ngMock change

@caitp caitp added this to the 1.4.0-beta.3 / 1.3.12 milestone Jan 27, 2015
@caitp caitp self-assigned this Jan 27, 2015
@lgalfaso
Copy link
Contributor

we can handle the done parameter in the wrapper --- but who cares about
jasmine 2.x anyways?

Angular/ngMock is also used by developers to test their modules, not only
to test angular itself

@petebacondarwin
Copy link
Member

For Jasmine 2, the spec functions can take an optional done fn parameter, yes?

If we were to migrate to supporing Jasmine 2 (which is not trivial for ngMock) then we would define the spec function for they blocks would take an optional second parameter:

describe( ... ) {
  it('should do something async', function(done) {
    ...
    done();
  };

  var vals = [ ... ];
  they('should do something async', function(vals, function(val, done) {
    ...
    done();
  });

Is there anything else we are missing here? If not then we should merge this once the other comments are addressed.

@lgalfaso
Copy link
Contributor

@petebacondarwin yes, and ok

@petebacondarwin petebacondarwin removed this from the 1.4.0-beta.5 / 1.3.14 milestone Feb 24, 2015
@matsko matsko closed this in e650c45 Mar 4, 2015
petebacondarwin added a commit that referenced this pull request Mar 4, 2015
There are now three new test helpers: `they`, `tthey` and `xthey`, which
will create multiple `it`, `iit` and `xit` blocks, respectively, parameterized
by each item in a collection that is passed.

(with tests and ammendments by @petebacondarwin)

Closes #10864
hansmaad pushed a commit to hansmaad/angular.js that referenced this pull request Mar 10, 2015
There are now three new test helpers: `they`, `tthey` and `xthey`, which
will create multiple `it`, `iit` and `xit` blocks, respectively, parameterized
by each item in a collection that is passed.

(with tests and ammendments by @petebacondarwin)

Closes angular#10864
netman92 pushed a commit to netman92/angular.js that referenced this pull request Aug 8, 2015
There are now three new test helpers: `they`, `tthey` and `xthey`, which
will create multiple `it`, `iit` and `xit` blocks, respectively, parameterized
by each item in a collection that is passed.

(with tests and ammendments by @petebacondarwin)

Closes angular#10864
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.

6 participants