Skip to content
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

feat(core): support global fakeAsync functionality across multiple test cases. #40611

Draft
wants to merge 2 commits into
base: master
from

Conversation

@JiaLiPassion
Copy link
Contributor

@JiaLiPassion JiaLiPassion commented Jan 28, 2021

Close #40387

Enable global fakeAsync() feature by introducing two new functions.
beginFakeAsync() and endFakeAsync().

Motivation

Consider the similar function from jest.useFakeTimers(), we can write test cases like this.

describe('test', () => {
   beforeAll(() => {
     jest.useFakeTimers();   
   });

   afterAll(() => {
     jest.clearAllTimers();
     jest.useRealTimers();
   });

   test('async test1', () => {
      setInterval(() => {}, 100);
      jest.advanceTimersByTime(100);
      expect(...);
   });

    test('async test2', () => {
      setInterval(() => {}, 200);
      jest.advanceTimersByTime(200);
      expect(...);
   });
});

So with jest.useFakeTimers(), all the following tests are automatically using the fake timers, and the developers can write the common init, cleanup logic in beforeEach()/afterEach().

But with fakeAsync(), we can not do the same thing for now.

For example, we have a Component like this.

@component({...})
export class AppComponent {
  timerId: number;
  ngOnInit() {
    this.timerId = setTimeout(() => {});
  }

  ngOnDestroy() {
    clearTimeout(this.timerId);
  }
}

And for now, we need to write test
like this.

describe('AppComponent test', () => {
  let fixture: ComponentFixture<AppComponent>;
  beforeEach(() => {
    ...
    fixture = TestBed.createComponent(AppComponent);
  });

  it('test case1', fakeAsync(() => {
    fixture.detectChanges();
    // do some test with fixture
    fixture.destroy();
  }));

  it('test case2', fakeAsync(() => {
    fixture.detectChanges();
    // do some test with fixture
    fixture.destroy();
  }));
});

We need to call fixture.destroy() inside each tests, since each it() use
it's own fakeAsync() and we need to clean up the timerId created in that
FakeAsyncZoneSpec. Otherwise the fakeAsync() will throw there are still pending timers error.

Solution

So in this PR, there are two functions are introduced.

  • beginFakeAsync()
    begin fakeAsync globally, so we can use share one FakeAsyncZoneSpec
    instance across multiple test cases.

With this feature, we can do some common cleanup in afterEach().

With the beginFakeAsync(), we can write case in this way.

describe('AppComponent test', () => {
  let fixture: ComponentFixture<AppComponent>;

  beforeAll(() => {
    beginFakeAsync();
  });
  
  afterAll(() => {
    endFakeAsync();
  });

  beforeEach(() => {
    ...
    fixture = TestBed.createComponent(AppComponent);
  });

  afterEach(() => {
    fixture.destroy();
  });

  it('test case1', () => {
    fixture.detectChanges();
    // do some test with fixture
  });

  it('test case2', () => {
    fixture.detectChanges();
    // do some test with fixture
  });
});

This feature will be useful when we want to do some global cleanup
in afterEach() when the component have some async tasks
(especially setInterval) running by 3rd party library.

@JiaLiPassion JiaLiPassion requested a review from mhevery Jan 28, 2021
@ngbot ngbot bot added this to the Backlog milestone Jan 28, 2021
@google-cla google-cla bot added the cla: yes label Jan 28, 2021
@pullapprove pullapprove bot requested a review from IgorMinar Jan 28, 2021
@JiaLiPassion JiaLiPassion marked this pull request as draft Jan 29, 2021
@mhevery
Copy link
Member

@mhevery mhevery commented Jan 29, 2021

What happens when developer forgets to invoke endFakeAsync()? Is there a sane failure mode?

An alternative approach may be something as shown below. Have you consider it? What do you think are advantages/disadvantages of the two approaches?

describe('AppComponent test', () => {
  let fixture: ComponentFixture<AppComponent>;
  const withFixtureFakeAsync = fakeAsync.bind({
    before:  () => fixture = TestBed.createComponent(AppComponent),
    after: () => fixture.destroy()
  });

  it('test case1', withFixtureFakeAsync(() => {
    fixture.detectChanges();
    // do some test with fixture
  }));

  it('test case2', withFixtureFakeAsync(() => {
    fixture.detectChanges();
    // do some test with fixture
  }));
});

@JiaLiPassion
Copy link
Contributor Author

@JiaLiPassion JiaLiPassion commented Jan 30, 2021

@mhevery, yeah, I like your idea,

  1. This is much safer, the users don't need to worry about forgetting to call endFakeAsync().
  2. And the user will know that their tests are under fakeAsync(), also in the same describe, they can write fakeAsync(), async(), normal test together.

I will update the PR

@JiaLiPassion JiaLiPassion force-pushed the JiaLiPassion:global-fakeasync branch from ee3ccbb to b599041 Jan 31, 2021
@JiaLiPassion JiaLiPassion force-pushed the JiaLiPassion:global-fakeasync branch from b599041 to c36741a Feb 1, 2021
@petebacondarwin
Copy link
Member

@petebacondarwin petebacondarwin commented Feb 1, 2021

By the way, should the hooks be beforeEach and afterEach to be more inline with the jasmine API?

@JiaLiPassion JiaLiPassion force-pushed the JiaLiPassion:global-fakeasync branch from c36741a to 17582fd Feb 9, 2021
@JiaLiPassion
Copy link
Contributor Author

@JiaLiPassion JiaLiPassion commented Feb 9, 2021

@petebacondarwin , yes, you are right, beforeEach() and afterEach() are better names, I updated the PR, thank you.

@JiaLiPassion JiaLiPassion force-pushed the JiaLiPassion:global-fakeasync branch 3 times, most recently from 72fba02 to 56fd0d6 Feb 9, 2021
…ach()` hooks functions.

Close #40387

Wrap the `fakeAsync()` with the `beforeEach()/afterEach()` hooks.

For example, we have a Component like this.

@component({...})
export class AppComponent {
  timerId: number;
  ngOnInit() {
    this.timerId = setTimeout(() => {});
  }

  ngOnDestroy() {
    clearTimeout(this.timerId);
  }
}

And without hook functions, we need to write test
like this.

describe('AppComponent test', () => {
  let fixture: ComponentFixture<AppComponent>;
  beforeEach(() => {
    ...
    fixture = TestBed.createComponent(AppComponent);
  });

  it('test case1', fakeAsync(() => {
    fixture.detectChanges();
    // do some test with fixture
    fixture.destroy();
  }));

  it('test case2', fakeAsync(() => {
    fixture.detectChanges();
    // do some test with fixture
    fixture.destroy();
  }));
});

We need to call `fixture.destroy()` inside each tests, since each `it()` use
it's own fakeAsync() and we need to clean up the timerId created in that
FakeAsyncZone.

With the hook functions, we can write case in this way.

describe('AppComponent test', () => {
  let fixture: ComponentFixture<AppComponent>;

  const fakeAsyncWithFixture = fakeAsync.wrap({
    beforeEach: () => {
      fixture = TestBed.createComponent(AppComponent);
      fixture.detectChanges();
    }
    afterEach: () => fixture.destroy();
  });

  it('test case1', fakeAsyncWithFixture(() => {
    // do some test with fixture
  }));

  it('test case2', fakeAsyncWithFixture(() => {
    // do some test with fixture
  }));
});

Also the wrap() function support nesting.

describe('AppComponent test', () => {
  let fixture: ComponentFixture<AppComponent>;

  const fakeAsyncWithFixture = fakeAsync.wrap({
    beforeEach: () => {
      fixture = TestBed.createComponent(AppComponent);
      fixture.detectChanges();
    }
    afterEach: () => fixture.destroy();
  });

  it('test case1', fakeAsyncWithFixture(() => {
    // do some test with fixture
  }));

  it('test case2', fakeAsyncWithFixture(() => {
    // do some test with fixture
  }));

  describe('AppComponent sub test: auth test', () => {
    const fakeAsyncNested = fakeAsyncWithFeature.wrap({
      beforeEach: () => fixture.componentInstance.login(),
      afterEach: () => fixture.componentInstance.logout();
    });

    it('should show user info', () => {
      // do some test with fixture with authenticated user.
    });
  });
});

This feature is useful when we want to do some common initial/cleanup when the component has some `async`
tasks (especially `setInterval`) running by 3rd party library.
@JiaLiPassion JiaLiPassion force-pushed the JiaLiPassion:global-fakeasync branch 2 times, most recently from d34964d to fe770f6 Feb 10, 2021
@mary-poppins
Copy link

@mary-poppins mary-poppins commented Feb 10, 2021

In the previous commit, `zone.js` provides a new API `fakeAsync.wrap()`
to support `hooks` of `fakeAsync()` to allow user to write common `initial`/'cleanup`
logic. This commit add this new `wrap()` API to `@angular/core/testing`.
@JiaLiPassion JiaLiPassion force-pushed the JiaLiPassion:global-fakeasync branch from fe770f6 to 0f8d2ac Feb 10, 2021
@mary-poppins
Copy link

@mary-poppins mary-poppins commented Feb 10, 2021

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants