feat(core): support fakeAsync.wrap() functionality across multiple test cases.

[JiaLiPassion]

Close #40387

Enable global fakeAsync() feature by introducing hooks functions.
fakeAsync.wrap({beforeEach?: () => void, afterEach?: () => void);

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 hooks functions are introduced.

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 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.

[mhevery]

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]

@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

[petebacondarwin]

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

[JiaLiPassion]

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

[mary-poppins]

You can preview fe770f6 at https://pr40611-fe770f6.ngbuilds.io/.

[mary-poppins]

You can preview 0f8d2ac at https://pr40611-0f8d2ac.ngbuilds.io/.

[mary-poppins]

You can preview 153891e at https://pr40611-153891e.ngbuilds.io/.

[dylhunn]

reviewed-for: public-api

[atscott]

Hi @JiaLiPassion, it looks like the issue you linked was closed by 0652b29. Is this still necessary?

That question aside, I'm not sure I see why this specific thing should be a feature provided by the core/zone/fakeAsync API. How is it different from this:

  describe('AppComponent test', () => {
     let fixture: ComponentFixture<AppComponent>;
   
     beforeEach(() => {
       fixture = TestBed.createComponent(AppComponent);
       fixture.detectChanges();
     });
     afterEach(() => fixture.destroy());
   
     it('test case1', fakeAsync(() => {
       // do some test with fixture
     }));
   
     it('test case2', fakeAsync(() => {
       // do some test with fixture
     }));
   });

The above is actually shorter than your example using this new feature.

[JiaLiPassion]

@atscott thank you for the review.
The reason we need a new wrap API is to make sure before/after hook can run in the same fakeAsync() zone with the it() test case.
If we write case like this,

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

  ngOnDestroy() {
    clearTimeout(this.timerId);
  }
}
  describe('AppComponent test', () => {
     let fixture: ComponentFixture<AppComponent>;
   
     beforeEach(() => {
       fixture = TestBed.createComponent(AppComponent);
       fixture.detectChanges();
     });
     afterEach(() => fixture.destroy());
   
     it('test case1', fakeAsync(() => {
       // do some test with fixture
      tick();
     }));
   
     it('test case2', fakeAsync(() => {
       // do some test with fixture
     }));
   });

the setTimeout in beforeEach() will not in the fakeAsync() and will run asynchronize, so the it('test case1') and it('test case2') will run before the setTimeout callback executed, and calling tick() in it() will not trigger the setTimeout in the beforeEach() which may not match the user's expection.

[atscott]

@JiaLiPassion Okay, I think that makes sense. Could you please update the documentation to describe this better? The way it is now, it seems to imply what I wrote should work fine. "without hook functions, we need to write test like this" Unless I'm missing something, the example after that sentence is exactly what I wrote in my comment.

Also, I'm still curious about 0652b29 and if it makes this PR obsolete. If not, the commit message should be changed to reflect that this solves a different issue (one that's not closed).

[atscott]

Is the example code supposed to be here rather than fake-async-test.ts? Or should it be in both locations

[JiaLiPassion]

@atscott , got it, I will update the commit message, and about the aio doc, in the previous commit, the aio document and the sample code are also be updated, but it seems the aio project reference the 13.2.2 version instead of the local version, so the sample code will not be compiled since the new wrap() API.

[atscott]

@JiaLiPassion I've moved this back to a draft since it doesn't seem ready for review

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}