Skip to content

regression: jasmine.clock() inside fakeAsync() doesn't work anymore since 0.8.21 #31708

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
jnizet opened this issue Mar 31, 2018 · 28 comments
Closed
Assignees
Labels
area: zones Issues related to zone.js P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent state: needs more investigation
Milestone

Comments

@jnizet
Copy link
Contributor

jnizet commented Mar 31, 2018

This happens in an Angular application.

Simple steps to reproduce

  • install Angular CLI
  • generate a project with ng new
  • write the following clock.spec.ts test
import { fakeAsync } from '@angular/core/testing';

fdescribe('clock test', () => {

  afterEach(() => jasmine.clock().uninstall());

  it('should fake the clock in fakeAsync', fakeAsync(() => {
    const fakeTime = 12345678;
    jasmine.clock().mockDate(new Date(fakeTime));

    expect(new Date().getTime()).toBe(fakeTime);
  }));
});

Run ng test to execute the test. It fails when zone.js version 0.8.21 is being used (default dependency to zone.js in package.json is ^0.8.19, which now resolves to 0.8.21. If you change it to 0.8.20 and execute npm installor yarn install to make sure the previous version of zone.js is used, the test passes.

@JiaLiPassion
Copy link
Contributor

@jnizet , in 0.8.21, if you use jasmine.clock(), you don't need to call fakeAsync, zone.js will call it for you. could you try

it('should fake the clock in fakeAsync', () => {
    const fakeTime = 12345678;
    jasmine.clock().mockDate(new Date(fakeTime));

    expect(new Date().getTime()).toBe(fakeTime);
  });

@jnizet
Copy link
Contributor Author

jnizet commented Mar 31, 2018

@JiaLiPassion this is a dumb-down version of my real test. In the actual test, I also use tick(). In fact I use this helper class:

class FakeNow {
  private now = DateTime.local(); // this is just luxon delegating to new Date()

  constructor() {
    jasmine.clock().mockDate(this.now.toJSDate());
  }

  tickSeconds(seconds: number) {
    this.now = this.now.plus({ seconds });
    jasmine.clock().mockDate(this.now.toJSDate());
    tick(seconds * 1000);
  }
}

And call fakeNow.tickSeconds() several times in my test. Everything works fine in 0.8.20.

Since 0.8.21, this doesn't work anymore.

Based on your comments, I tried:

  • to remove the fakeAsync() wrapper as you suggest, and leave the FakeNow class as is: angular complains that tick() shouldn't be called outside of the fakeAsync zone
  • to remove the fakeAsync() wrapper as you suggest, and remove the call to tick(seconds * 1000) in FakeNow: doesn't work, because I use RxJS interval() in my code, and without the call to tick(), the interval observable doesn't emit anything.

Please tell me if you need more information, and when I have some time, I'll try to provide a reproduction of the actual problem.

@JiaLiPassion
Copy link
Contributor

@jnizet , please provide a reproduce repo, in new version, you don't need to Fakenow yourself.
But current version have some issue, it will be fixed in next version, but if you can provide a reproduce repo, I can verify my fix is correct or not. thank you.

@jnizet
Copy link
Contributor Author

jnizet commented Mar 31, 2018

@JiaLiPassion Here's a repo reproducing the issue: https://github.com/jnizet/zonejsbug

The bug is illustrated in https://github.com/jnizet/zonejsbug/blob/master/src/app/clock.spec.ts.

Switching back to 0.8.20 in package.json, then executing yarn, and re-executing ng test shows that the test passes with 0.8.20.

The app.component.spec.ts file illustrates another regression, for which I'll post another issue.

@JiaLiPassion
Copy link
Contributor

@jnizet, thank you for posting the issue, I will post a sample after #23108 issue is fixed.

@jnizet
Copy link
Contributor Author

jnizet commented Mar 31, 2018

OK, thanks.

BTW, I had the impression that 0.8.21 was supposed to support the RxJS delay operator (see #10127)

but this simple test doesn't pass:

import { fakeAsync, tick } from '@angular/core/testing';
import { delay } from 'rxjs/operators';
import { of } from 'rxjs/observable/of';

describe('delay', () => {
  it('should support delay', fakeAsync(() => {
    let done = false;

    const obs = of(true).pipe(
      delay(300)
    ).subscribe(() =>{
      done = true;
    });

    // window.setTimeout(() => done = true, 300);

    expect(done).toBe(false);
    tick(350);
    expect(done).toBe(true);
  }));
});

and I haven't been able to understand how to use your example at #10127 (comment). Native support, without having to import a ts file out of the project, would be nice.

@JiaLiPassion
Copy link
Contributor

@jnizet , you need to import an additional file import 'zone.js/dist/zone-patch-rxjs-fake-async' to make rxjs/scheduler/asap/async to support fakedate. I will post a sample based on your repo after those issues are stable.

@JiaLiPassion
Copy link
Contributor

@jnizet, please update to zone.js 0.8.25, it will work the same as 0.8.20.

And I also create a sample here ,JiaLiPassion/zonejsbug@97c58f9

which describes that you can also test like this, because angular not released yet, so it need some additional setup. you can choose to use old way or the new method.

@jnizet
Copy link
Contributor Author

jnizet commented Apr 4, 2018

@JiaLiPassion thanks for looking into this. The reproduction test I gave you now doesn't fail anymore, but unfortunately, I must have oversimplified the actual test a little, because my actual test still doesn't pass:

  • leaving the code as it is still fails, but a bit differently from before.
  • trying to apply the changes in your example fails too: the tests complain that tick() should be called inside the fakeAsync zone, or that fakeAsync() calls must not be nested. Even disabled tests complain.

To help figuring out the actual issue, it would be great if you could look into the actual test case (which is a bit more complex than the repro I gave before, but not that much).

Here's the github repo: https://github.com/Ninja-Squad/globe42

To reproduce the issue, you'll have to:

  • clone the repo
  • git checkout renovate/zone.js-0.x
  • cd frontend
  • yarn
  • ng test

The version of zone.js in this branch is 0.8.25, and the problematic test uses fdescribe(), so only that test should be executed.

This test passed fine before, when using version 0.8.20 (which you can check by checkouting the master branch, or just changing the version in package.json, if you want to).

@JiaLiPassion
Copy link
Contributor

@jnizet, sure, I have created a commit here, JiaLiPassion/globe42@31b1151

with new version of zone.js, your test code can be much simpler , you don't need to fakeDate yourself. and you event don't need jasmine.clock() in your case.

And after angular new version released, the setup code can also be removed, JiaLiPassion/globe42@31b1151#diff-1cceb3e8058181e7f0a7609600970077R10
you can just import async, fakeAsync, tick from @angular/core/testing like always.

@jnizet
Copy link
Contributor Author

jnizet commented Apr 4, 2018

Thanks a lot for your help @JiaLiPassion.

By "next version of Angular", do you mean the next minor/patch 5.x version, or the next major 6.0 version?

@JiaLiPassion
Copy link
Contributor

@jnizet, I am not sure which version is, I believe it will be 6.0. because #23108 is master-only.

if it will cost some time to upgrade to angular 6, you can still use angular 5 and newest version of zone.js by using the method described here JiaLiPassion/globe42@31b1151

@jnizet
Copy link
Contributor Author

jnizet commented Apr 4, 2018

Thanks. I definitely plan to migrate ASAP, and I've already prepared the migration in a different branch. Just wanted to know. Thanks again.

@JiaLiPassion
Copy link
Contributor

@jnizet, glad to help!

@jnizet
Copy link
Contributor Author

jnizet commented Apr 4, 2018

@JiaLiPassion you not only helped me and the rest of the Angular community, but you also indirectly helped a small local non-profit organization that cares about old, poor immigrants. 👏

@JiaLiPassion
Copy link
Contributor

@jnizet , thank you! very glad and proud to know that!

@insidewhy
Copy link

jasmine.clock().mockDate(...); isn't working at all for me, still getting the "current time" instead of what I pass to mockDate. Was working fine before upgrading to angular 6.

@JiaLiPassion
Copy link
Contributor

@ohjames, please provide a reproduce repo .

@insidewhy
Copy link

@JiaLiPassion The reproduction is simple, merely call jasmine.clock().mockDate(...) inside of your beforeEach along with your jasmine.clock().install() and it has no effect. You have to call it inside of the test now, but if you have say 100 tests grouped in the same describe block which should all use the same mocked date, now you have one hundred extra lines in your source code.

@JiaLiPassion
Copy link
Contributor

@ohjames , you mean

beforeEach(() => {
  jasmine.clock().install();
  jasmine.clock().mockDate(2017, 1,1);
});

it (() => {
  expect(Date.now.getFullYear()).toBe(2017); // will fail
});

this case will fail,

beforeEach(() => {
  jasmine.clock().install();
});

it (() => {
  jasmine.clock().mockDate(2017, 1,1);
  expect(Date.now.getFullYear()).toBe(2017); // will success
});

this will success.

is that right ?
And you are using fakeAsync?

Please paste some sample test codes here, it will help a lot.

@insidewhy
Copy link

I ported all our tests over to fakeAsync and use tick() with a negative number to set the time to what I need: tick(desiredTime - Date.now()) so I'm not worried about this issue anymore, but I suppose someone else will report it.

@JiaLiPassion
Copy link
Contributor

@ohjames , thanks for the update, I will check jasmine.clock with Date in beforeEach.

@Teamop
Copy link
Contributor

Teamop commented Aug 17, 2018

@JiaLiPassion
I found an interesting issue with jasmine.clock with beforeAll and fakeAsync.
I just use this specs fakeAsyncTest should work without patch jasmine.clock(Line 968) in the test\zone-spec\fake-async-test.spec.ts to reproduce the issue.

then just add one test case at the first after the beforeEach and AfterEach

it('should mock date correctly', () => {
  const result = Date.now();
  console.log(new Date(result));
  expect(result).toEqual(baseTime.getTime());
});

Then can reproduce as following(with changes just for the beforeEach, AfterEach and the test case above(not focused)):

// change the 'beforeEach' to 'beforeAll', 'afterEach' to 'afterAll', then the added test case will fail because the date isn't mocked
beforeAll(() => {
    spy = jasmine.createSpy('timer');
    jasmine.clock().install();
    baseTime = new Date(2013, 9, 23);
    jasmine.clock().mockDate(baseTime);
 });

 afterAll(() => {
    jasmine.clock().uninstall();
 });

then move the jasmine.clock().mockDate(baseTime) to the added case, the added case will be passed. like this:

beforeAll(() => {
 spy = jasmine.createSpy('timer');
 jasmine.clock().install();
 baseTime = new Date(2013, 9, 23);
});

afterAll(() => {
 jasmine.clock().uninstall();
});

it('should mock date correctly', () => {
 jasmine.clock().mockDate(baseTime);           // move to this place
 const result = Date.now();
 console.log(new Date(result));
 expect(result).toEqual(baseTime.getTime());
});

While, if with beforeEach and AfterEach, this case can always pass no matter the position of this jasmine.clock().mockDate()

But when I want to test the added spec only with 'fit', this added spec can always pass, but if with one case with fakeAsync, just the one after the added spec. This added spec sometimes pass and sometimes fail.

@insidewhy
Copy link

@Teamop see my previous comment, you can't use those methods anymore. Write your own that uses tick with a negative number to reach your target date. It's simple. I went through this pain so you don't have to :P

@Teamop
Copy link
Contributor

Teamop commented Aug 17, 2018

@ohjames yes, but for me, this just happens with beforeAll and afterAll with fakeAsync, besides, just want to elaborate on them to provide some information. Hope it can help to identity the real issue.

@insidewhy
Copy link

Yeah it can't be used in any hook other than the test itself since angular 6.

@JiaLiPassion
Copy link
Contributor

@ohjames, @Teamop, I will check this issue again, thank you for posting the issue.

@JiaLiPassion JiaLiPassion transferred this issue from angular/zone.js Jul 21, 2019
@JiaLiPassion JiaLiPassion self-assigned this Jul 21, 2019
@JiaLiPassion JiaLiPassion added the area: zones Issues related to zone.js label Jul 21, 2019
@ngbot ngbot bot added this to the needsTriage milestone Jul 21, 2019
@jelbourn jelbourn added the P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent label Nov 2, 2020
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Nov 2, 2020
@alxhub
Copy link
Member

alxhub commented Apr 24, 2025

Closing as obsolete - there's been no activity on this issue since 2018, and it's unlikely we'll make any changes to the Zone Jasmine patch at this point. If you're still encountering this problem, feel free to open a new issue with a reproduction.

@alxhub alxhub closed this as not planned Won't fix, can't repro, duplicate, stale Apr 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: zones Issues related to zone.js P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent state: needs more investigation
Projects
None yet
Development

No branches or pull requests

6 participants