chore(*): Upgrade to Jasmine 2.4 #14226

lgalfaso · 2016-03-13T12:24:15Z

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Chore

What is the current behavior? (You can also link to an open issue here)
All tests run with Jasmine 1.3

What is the new behavior (if this is a feature change)?
All tests run with Jasmine 2.4

Does this PR introduce a breaking change?
No

Please check if the PR fulfills these requirements

The commit message follows our guidelines: https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#commit-message-format
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

Other information:

Highlights:
in Jasmine, this is no longer jasmine.Spec. Removed the tests that check this.
New mechanism to run async tests as Jasmine 2 removed runs, waits and waitsFor
The functions iit, ddescribe and tthey were renamed fit, fdescribe and fthey
as the originals came from Karma, Karma no longer bundles Jasmine and the new function
name comes from Jasmine
7e91645 was partially reverted. Kept the test, remove the code changes

lgalfaso · 2016-03-13T12:27:23Z

This PR is big and given that it changes many files, can get easily rotten, so it would be ideal if this can be reviewed fast.

lgalfaso · 2016-03-13T12:28:26Z

There is a pending task to upgrade grunt-ddescribe-iit to support for fthey

lgalfaso · 2016-03-13T14:04:57Z

I do not have access to IE9, can someone check why there is an error when running tests:modules ?

Narretz · 2016-03-14T14:28:31Z

@lgalfaso The complete log shows an error for FIrefox in tests:modules:

Firefox 43.0.0 (Linux 0.0.0) ERROR
  Error: [ng:btstrpd] App Already Bootstrapped with this Element '&lt;div ng-app=""&gt;'
  http://errors.angularjs.org/1.5.1-build.20899+sha.0082806/ng/btstrpd?p0=%26lt%3Bdiv%20ng-app%3D%22%22%26gt%3B
  at /home/travis/build/angular/angular.js/build/angular.js:68
.................
Firefox 43.0.0 (Linux 0.0.0): Executed 17 of 713 ERROR (0.126 secs / 0.027 secs)

IE 9 is fine:

IE 9.0.0 (Windows 7 0.0.0): Executed 713 of 713 SUCCESS (2.888 secs / 2.348 secs)

Narretz · 2016-03-14T14:30:19Z

Why was lgalfaso@7e91645 reverted? Because we don't support these browsers?

lgalfaso · 2016-03-14T15:18:51Z

Why was lgalfaso/angular.js@7e91645 reverted? Because we don't support these browsers?

Jasmine 2.4 toEqual checks that actual and expected are of the same type. 7e91645 changes the type of what is thrown.

Narretz · 2016-03-14T15:32:20Z

Ok, thanks. Can you add that explanation to the commit message?

petebacondarwin · 2016-03-14T16:29:02Z

test/loaderSpec.js

+      ['$filterProvider', 'register', createArgumentsOf(['f', 'ff'])],
+      ['$compileProvider', 'directive', createArgumentsOf(['d', 'dd'])],
+      ['$compileProvider', 'component', createArgumentsOf(['c', 'cc'])],
+      ['$controllerProvider', 'register', createArgumentsOf(['ctrl', 'ccc'])]


Is it possible to just use jasmine.objectContaining for these tests?

petebacondarwin · 2016-03-14T17:25:30Z

@lgalfaso can you confirm that after the change to the angular mock module tests still work on PhantomJS and Safari 7? This is the only part that affects people outside of this project so it would be good to make sure we are not breaking anything.

Highlights: New mechanism to run async tests as Jasmine 2 removed `runs`, `waits` and `waitsFor` The functions `iit`, `ddescribe` and `tthey` were renamed `fit`, `fdescribe` and `fthey` as the originals came from Karma, Karma no longer bundles Jasmine and the new function name comes from Jasmine

lgalfaso · 2016-03-14T21:32:26Z

Reverted the changes on angular.mocks.js, made the changes on loaderSpec.js, fixed the issue with Firefox and now the test are back to green.

PTAL

Narretz · 2016-03-15T12:34:41Z

test/helpers/privateMocksSpec.js

@@ -9,7 +9,7 @@ describe('private mocks', function() {
        spyOn(window, 'it');

        they('should do stuff with $prop', ['a', 'b', 'c']);
-        expect(window.it.calls.length).toEqual(3);


All of these could be converted instead to .calls.count()).toEqual(... This would make it similar to what is was before.

I still think the new way is better and the way that is present in Jasmine documentation

Narretz · 2016-03-15T12:42:57Z

Since we now officially move to jasmine 2.4, I think we should also change the docs to recommend jasmine 2.4 instead of 1.3, espepcially here: https://github.com/angular/angular.js/blob/a4e60cb6970d8b6fa9e0af4b9f881ee3ba7fdc99/docs/content/guide/unit-testing.ngdoc and https://github.com/angular/angular.js/blob/a4e60cb6970d8b6fa9e0af4b9f881ee3ba7fdc99/docs/content/guide/e2e-testing.ngdoc The unit tests in the guide don't actually run, so they have to be checked for correctness with jasmine 2.4. @lgalfaso do you want to make these changes? I can also do it if you want.

lgalfaso · 2016-03-15T13:50:13Z

Updated the docs, did not run the tests in the guide, but in theory it
should be possible for users to use the version from Jasmine that they like
the most.

On Tue, Mar 15, 2016 at 1:43 PM, Martin Staffa [email protected]
wrote:

Since we now move to jasmine 2.4, I think we should also change the docs
to recommend jasmine 2.4 instead of 1.3, espepcially here:
https://github.com/angular/angular.js/blob/a4e60cb6970d8b6fa9e0af4b9f881ee3ba7fdc99/docs/content/guide/unit-testing.ngdoc
and
https://github.com/angular/angular.js/blob/a4e60cb6970d8b6fa9e0af4b9f881ee3ba7fdc99/docs/content/guide/e2e-testing.ngdoc
The unit tests in the guide don't actually run, so they have to be checked
for correctness with jasmine 2.4. @lgalfaso https://github.com/lgalfaso
do you want to make these changes? I can also do it if you want.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#14226 (comment)

Highlights: New mechanism to run async tests as Jasmine 2 removed `runs`, `waits` and `waitsFor` The functions `iit`, `ddescribe` and `tthey` were renamed `fit`, `fdescribe` and `fthey` as the originals came from Karma, Karma no longer bundles Jasmine and the new function name comes from Jasmine. Closes #14226

googlebot added the cla: yes label Mar 13, 2016

lgalfaso force-pushed the jasmine-24 branch from 94ce5d1 to 4218adc Compare March 13, 2016 12:40

lgalfaso assigned petebacondarwin Mar 14, 2016

lgalfaso force-pushed the jasmine-24 branch from 4218adc to c891bb5 Compare March 14, 2016 15:22

petebacondarwin reviewed Mar 14, 2016
View reviewed changes

lgalfaso force-pushed the jasmine-24 branch from c891bb5 to e862825 Compare March 14, 2016 21:16

Narretz reviewed Mar 15, 2016
View reviewed changes

Narretz added type: test type: chore labels Mar 15, 2016

petebacondarwin closed this in eb16611 Mar 16, 2016

gkalpak mentioned this pull request Mar 18, 2016

test(helpers): fix error message generation for toHaveBeenCalledOnce[With] matchers #14275

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(*): Upgrade to Jasmine 2.4 #14226

chore(*): Upgrade to Jasmine 2.4 #14226

lgalfaso commented Mar 13, 2016

lgalfaso commented Mar 13, 2016

lgalfaso commented Mar 13, 2016

lgalfaso commented Mar 13, 2016

Narretz commented Mar 14, 2016

Narretz commented Mar 14, 2016

lgalfaso commented Mar 14, 2016

Narretz commented Mar 14, 2016

petebacondarwin Mar 14, 2016

petebacondarwin commented Mar 14, 2016

lgalfaso commented Mar 14, 2016

Narretz Mar 15, 2016

lgalfaso Mar 15, 2016

Narretz commented Mar 15, 2016

lgalfaso commented Mar 15, 2016

chore(*): Upgrade to Jasmine 2.4 #14226

chore(*): Upgrade to Jasmine 2.4 #14226

Conversation

lgalfaso commented Mar 13, 2016

lgalfaso commented Mar 13, 2016

lgalfaso commented Mar 13, 2016

lgalfaso commented Mar 13, 2016

Narretz commented Mar 14, 2016

Narretz commented Mar 14, 2016

lgalfaso commented Mar 14, 2016

Narretz commented Mar 14, 2016

petebacondarwin Mar 14, 2016

Choose a reason for hiding this comment

petebacondarwin commented Mar 14, 2016

lgalfaso commented Mar 14, 2016

Narretz Mar 15, 2016

Choose a reason for hiding this comment

lgalfaso Mar 15, 2016

Choose a reason for hiding this comment

Narretz commented Mar 15, 2016

lgalfaso commented Mar 15, 2016