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

fix(test): fix #1069, FakeDate should handle constructor parameter #1070

Merged
merged 1 commit into from
Apr 6, 2018

Conversation

JiaLiPassion
Copy link
Collaborator

@JiaLiPassion JiaLiPassion commented Apr 5, 2018

fix #1069.
fix #1071.

  • in fakeAsync, now Date is monkey-patched with FakeDate, FakeDate should handle parameters in constructor correctly, such as new Date(1995, 1), new Date(0).

  • should handle Date.UTC() in fakeAsync correctly

  • should handle Date.parse() in fakeAsync correctly

@mhevery , sorry this is a regression bug, please review and please check should we cut a new release or not.

@JiaLiPassion JiaLiPassion force-pushed the date-args branch 4 times, most recently from 38e6d52 to 8fa6f49 Compare April 5, 2018 12:03
d.setTime(FakeDate.now());
return d;
case 1:
return new OriginalDate(arguments[0]);
Copy link
Contributor

Choose a reason for hiding this comment

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

did you try new OriginalDate(...arguments) That would be nicer than having a big switch statemente.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mhevery , yes, this is a better way, I will change it , thank you.

@JiaLiPassion JiaLiPassion force-pushed the date-args branch 4 times, most recently from 0eaed3f to 887c8bc Compare April 6, 2018 00:30
@JiaLiPassion
Copy link
Collaborator Author

JiaLiPassion commented Apr 6, 2018

find another compatible issue.

  • if user use the new version of zone.js
  • and they don't set the flag fakeAsyncPatchLock to true, so they don't use auto fakeAsync feature, which is default behavior
  • And user still use fakeAsync to work with jasmine.clock().

such as the following sample :

describe('', () => {
  beforeEach(jasmine.clock().install);
  afterEach(jasmine.clock().uninstall);
});

it('user do fakeAsync themselves', fakeAsync(() => {
   // 1. user use jasmine.clock() 
  //  2. and don't use clock patch
  //  3. and user use fakeAsync themselves
 setTimeout(spy, 100);
 expect(spy).not.toHaveBeenCalled();
 jasmine.clock().tick(100);
 expect(spy).toHaveBeenCalled();
}));

This case will fail because jasmine.clock() will also patch setTimeout, so setTimeout will not trigger onScheduleTask. So in this PR, we we make sure if we are in fakeAsync, we will use zone.js patched setTimeout.

So the current behavior of jasmine.clock() is

it('', fakeAsync(() => {})) it('', () => {})
enable patch clock 1. use FakeDate of fakeAsync.
2. use zone.js patched setTimeout
3. jasmine.clock().mockDate() and tick() will delegate to fakeAsync.
will auto go into fakeAsync, behavior will be the same with left
disable patch clock same as above 1. use FakeDate of jasmine.
2. use patched setTimout of jasmine.
3. jasmine.clock().mockDate() and tick() will use jasmine original one

Conclusion, if testBody is inside of fakeAsync, we will use full package of fakeAsync method instead of jasmine.clock(), if we are not using fakeAsync, we also should make sure jasmine.clock() work as non patched.

other modifications.

  • don't patch jasmine.clock() method multiple times.

  • can handle jasmine.clock().mockDate() with empty parameters.

  • add more test cases.

@mhevery mhevery merged commit b3fdd7e into angular:master Apr 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fakeAsync breaks Date constructor FakeDate does not accept a timestamp value
3 participants