Support finishCallback and rootZoneStable in async test

[JiaLiPassion]

🚀 feature request

Relevant Package

This feature request is for zone.js

Description

A clear and concise description of the problem or missing capability...

the cases is from angular components, the following test are from @mmalerba.

fdescribe('repro', () => {
  beforeEach(async () => {
    await TestBed.configureTestingModule({
      declarations: [OutsideNgZoneTest]
    }).compileComponents();
  });

  # Passes
  it('done should not wait for task outside of zone', done => {
    const fixture = TestBed.createComponent(OutsideNgZoneTest);
    const el = fixture.debugElement.nativeElement;
    fixture.detectChanges();
    fixture.whenStable().then(() => {
      expect(el.classList.contains('timeout-done')).toBe(false);
      done();
    });
  });

  # Passes
  it('fakeAsync should wait for task outside of zone', fakeAsync(() => {
    const fixture = TestBed.createComponent(OutsideNgZoneTest);
    const el = fixture.debugElement.nativeElement;
    fixture.detectChanges();
    flush();
    expect(el.classList.contains('timeout-done')).toBe(true);
  }));

  # Fails
  it('async should wait for task outside of zone', async(() => {
    const fixture = TestBed.createComponent(OutsideNgZoneTest);
    const el = fixture.debugElement.nativeElement;
    fixture.detectChanges();
    fixture.whenStable().then(() => {
      expect(el.classList.contains('timeout-done')).toBe(true);
    });
  }));
});

@Component({template: ''})
export class OutsideNgZoneTest {
  constructor(el: ElementRef, ngZone: NgZone) {
    ngZone.runOutsideAngular(() =>
        setTimeout(() => el.nativeElement.classList.add('timeout-done')));
  }
}

For the 1st and 3rd cases, we want some mechanism to let user to easily test the async cases.

Describe the solution you'd like

If you have a solution in mind, please describe it.

• For the 1st case we may need some thing like
  rootZoneWhenStable().then() or asyncStable().then() to monitor not only ngZone but also the async tasks outside of ngZone.

• For the 3rd case, we need to add an option to async zone spec, to let user monitor when all async are done.

Describe alternatives you've considered

Have you considered any alternative solutions or workarounds?

No very easy workarounds for now.

[devversion]

If we would handle the first case by introducing something like rootZoneWhenStable, do we really need to handle the third case in the AsyncZoneSpec specifically? Sounds like we could just rely on rootZoneWhenStable as well (basically also for the second case).

cc. @mmalerba

[mmalerba]

Yeah if we have the first one that's sufficient for our use case in Angular Components. Though as a separate issue, it might be nice to do it automatically in async() tests so that they behave the same as fakeAsync() tests W.R.T. NgZone

[mmalerba]

I talked to @IgorMinar about this, he was opposed to changing how fixture.whenStable() works, but we discussed some options for adding a separate way to wait on the root zone:

1. Have a standalone function (e.g. whenRootZoneStable) that we import from @angular/core/testing (Igor is opposed to this one)
2. Have a method on the root zone (e.g. Zone.root.whenStable) that we can call
3. Change TestBed to add an extra zone between the root zone and NgZone (if it does not already do this) and then expose some method on the new zone

Would also like to get @mhevery's input on this

[JiaLiPassion]

@mmalerba, thanks for the information.
From my point of view.

1. fixture.whenStable() should not change, it should only be resolved when ngZone is stable.
2. Zone.root.whenStable or whenRootZoneStable maybe not a good idea either, because
   • we may don't want to expose root zone to users.
   • root zone should not do anything, user should not use root zone to monitor async tasks.
3. Add another nested zone between TestBed and NgZone, it will work, and I think it is exactly current async/fakeAsync is doing, we let user know that we want to use async/fakeAsync from @angular/core/testing to help users to write async tests, so I think

it ('auto monitor when all done', async(() => {
  component.doAsyncInsideNgZone();
  component.doAsyncOutsideNgZone();
  fixture.whenStable(() => {
    // check ngZone
 });
 asyncAllDone.whenStable(() => {
  // check ngZone and non ngZone
 });
}));

So I think we should only support such kind of features inside async/fakeAsync instead of providing users some magic behavior.
I know it will be a little weird that user write code like

it('async test', async(async () => {}));

the async test and javascript async will be confuse, so I also think maybe we should rename the async to asyncTest?

@mhevery , @IgorMinar , please review, thanks!

[devversion]

fixture.whenStable() should not change, it should only be resolved when ngZone is stable.

Yeah, I agree with that.

Zone.root.whenStable or whenRootZoneStable maybe not a good idea either, because we may don't want to
expose root zone to users. root zone should not do anything, user should not use root zone to monitor async tasks.

I'd personally expect no distinction between the root zone and others. Conceptually to me, the root zone is not any different to other zones, except that it just does not have any parent or spec.

Add another nested zone between TestBed and NgZone, it will work, and I think it is exactly current async/fakeAsync is doing, we let user know that we want to use async/fakeAsync from @angular/core/testing to help users to write async tests, so I think

Unfortunately this would not be sufficient as these interceptions are not necessarily only needed in pages using TestBed. For example, in our concrete use-case we also want to intercept the root zone in e2e selenium tests to monitor the page task count.

Currently we can already intercept individual zones by patching into Zone#_updateTaskCount. This works great so far, but the only issue here is that intercepting later means that previously scheduled tasks could be scheduled that we don't know about. For this, we currently need to load the patch immediately after ZoneJS loads, but it would be solved nicely if individual Zone's internally keep track of the task counts. It seems like logic for this partially already exists and it would be low-risk to keep track of the task count per zone, right?

[devversion]

Would there be a chance that ZoneJS adds logic to keep track of the tasks count regardless of whether there is a zone spec? The zone delegates already seem to have the capability to record the active tasks, but just due to the fact that the root zone does not have any spec applied, the delegate task count is not updated. This could be changed without a lot of effort, risk and performance costs I'd assume (even just an exception for the root zone delegate would be sufficient).

The only problem I see here is that this is not part of any public API so it seems a little unexpected. Having a larger and public API for retrieving/intercepting the task state is most likely more heavy though and might be out-of-scope for ZoneJS.

Note that adding simple APIs for intercepting, or just interposing a custom zone w/ spec is not sufficient. Those interceptors or custom zones are not guaranteed to know about tasks previously scheduled. Hence my proposal that Zone's keep track of the state internally by default. The interceptors could then register at any time without the risk of missing tasks scheduled before the interceptor initialized.

[angular-robot]

Just a heads up that we kicked off a community voting process for your feature request. There are 20 days until the voting process ends.

Find more details about Angular's feature request process in our documentation.

[angular-robot]

Thank you for submitting your feature request! Looks like during the polling process it didn't collect a sufficient number of votes to move to the next stage.

We want to keep Angular rich and ergonomic and at the same time be mindful about its scope and learning journey. If you think your request could live outside Angular's scope, we'd encourage you to collaborate with the community on publishing it as an open source package.

You can find more details about the feature request process in our documentation.

[jessicajaniuk]

@JiaLiPassion Do we plan to do anything with this?

[JiaLiPassion]

@jessicajaniuk , there is PR for this one, #45211, maybe we can discuss with the team whether this is still an issue in the angular component test.