tick and fakeAsync doesn't work well in Angular 13 with rxjs 7 debounceTime

[bgerstle-quot]

Which @angular/* package(s) are the source of the bug?

core

Is this a regression?

Yes

Description

I upgraded to angular 13 and then tried upgrading rxjs to 7. The problem is it seems rxjs has changed the way the debounceTime operator works: https://rxjs.dev/6-to-7-change-summary#debouncetime. Because of this a lot of my tests now fail because the fakeasync scope and debounceTime don't work well when the debounceTime is set up outside of a fakeAsync scope.
If you run the test in this file: https://github.com/bgerstle-quot/debounceTest/blob/master/src/app/app.component.spec.ts the first test passes and the second fails.

Please provide a link to a minimal reproduction of the bug

https://github.com/bgerstle-quot/debounceTest/blob/master/src/app/app.component.spec.ts

Please provide the exception or error you saw

No response

Please provide the environment you discovered this bug in (run ng version)

Angular CLI: 13.0.4
Node: 16.13.0
Package Manager: yarn 1.22.5
OS: linux x64

Angular: 13.0.3
... animations, common, compiler, compiler-cli, core, forms
... platform-browser, platform-browser-dynamic, router

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1300.4
@angular-devkit/build-angular   13.0.4
@angular-devkit/core            13.0.4
@angular-devkit/schematics      13.0.4
@angular/cli                    13.0.4
@schematics/angular             13.0.4
rxjs                            7.4.0
typescript                      4.4.4

Anything else?

No response

[JoostK]

The problem here is that the template is checked for changes outside of fakeAsync, which initializes the debounceTime operator upon receiving the value from the BehaviorSubject. At this time, debounceTime schedules a callback that is pending. When emitting a value from within the fakeAsync, the debounceTime operator realizes that it has already scheduled a timer and yields to that existing one. That is the optimalization implemented in RxJS 7, but it means that fakeAsync is never made aware of any scheduled action at all.

I don't see how this is an Angular bug, then. Since fakeAsync never sees any scheduling actions, it cannot possibly take over their execution.

To make the tests work, you'd have to defer the change detection cycle into the fakeAsync block.

[bgerstle-quot]

I moved the detectChanges into the tests and that wasn't enough. What did do it was changing the beforeEach into a callable function and then just running it at the start of each test. You can see what I'm talking about on this branch. It's definitely a usable workaround, but not ideal.
I also tried moving the fakeAsync to the describe, but causes an exception, which probably makes sense, but still prevents me from using the beforeEach
Angular consumes RxJS and fakeAsync is an angular function; therefore, I would argue that it is an Angular bug, without a fix, I can't use the beforeEach the way Angular intends it to be used.

[artaommahe]

@JoostK We have some of our test broken after update from ng12.1.1 to ng13.1.0. We use jest via @nrwl/jest builder

Reduced an issue to such case

test('smth', fakeAsync(() => {
  const data = new Subject<void>();
  const received = jest.fn();

  data
    .pipe(
      // rate limiting, no more than 10 events per second
      concatMap(update => concat(of(update), EMPTY.pipe(delay(100)))),
    )
    .subscribe(received);

  expect(received).toHaveBeenCalledTimes(0);

  data.next();
  data.next();

  expect(received).toHaveBeenCalledTimes(1);

  tick(50);
  expect(received).toHaveBeenCalledTimes(1);

  tick(51);
  expect(received).toHaveBeenCalledTimes(2);
}));

results in

    Expected number of calls: 1
    Received number of calls: 2

      336 |
      337 |         expect(received).toHaveBeenCalledTimes(1);
    > 338 |
          | ^
      339 |         tick(50);
      340 |         expect(received).toHaveBeenCalledTimes(1);
      341 |

so it fails even before first tick call. This reproduces with clear new nrwl/nx application (to have jest builder used)

in real tests we have stream initialisation separately in beforeEach block but looks like it's broken even within single fakeAsync call

[artaommahe]

@JoostK added a reproduction with clean ng13 install

• git clone git@github.com:artaommahe/ng13-tick-issue.git
• yarn install
• yarn test

/Users/drow/projects/tests/ng13-tick-issue % yarn test
yarn run v1.22.17
$ ng test
⠙ Generating browser application bundles (phase: setup)...29 12 2021 14:21:15.154:WARN [karma]: No captured browser, open http://localhost:9876/
29 12 2021 14:21:15.160:INFO [karma-server]: Karma v6.3.9 server started at http://localhost:9876/
29 12 2021 14:21:15.160:INFO [launcher]: Launching browsers Chrome with concurrency unlimited
29 12 2021 14:21:15.162:INFO [launcher]: Starting browser Chrome
✔ Browser application bundle generation complete.
29 12 2021 14:21:16.715:WARN [karma]: No captured browser, open http://localhost:9876/
29 12 2021 14:21:16.800:INFO [Chrome 96.0.4664.110 (Mac OS 10.15.7)]: Connected on socket ibPCfeTVPwEeAugXAAAB with id 77670425
✔ Browser application bundle generation complete.
Chrome 96.0.4664.110 (Mac OS 10.15.7) AppComponent smth FAILED
        Error: Expected spy unknown to have been called once. It was called 2 times.
            at <Jasmine>
            at UserContext.<anonymous> (src/app/app.component.spec.ts:21:22)
            at UserContext.fakeAsyncFn (node_modules/zone.js/fesm2015/zone-testing.js:1968:1)
            at ZoneDelegate.invoke (node_modules/zone.js/fesm2015/zone.js:372:1)
        Error: Expected spy unknown to have been called once. It was called 2 times.
            at <Jasmine>
            at UserContext.<anonymous> (src/app/app.component.spec.ts:24:22)
            at UserContext.fakeAsyncFn (node_modules/zone.js/fesm2015/zone-testing.js:1968:1)
            at ZoneDelegate.invoke (node_modules/zone.js/fesm2015/zone.js:372:1)
Chrome 96.0.4664.110 (Mac OS 10.15.7): Executed 1 of 1 (1 FAILED) (0.027 secs / 0.019 secs)
TOTAL: 1 FAILED, 0 SUCCESS
Chrome 96.0.4664.110 (Mac OS 10.15.7) ERROR

spec code same as above https://github.com/artaommahe/ng13-tick-issue/blob/master/src/app/app.component.spec.ts#L5. Worked fine with ng12.1

[Kiser360]

I encountered this after upgrading to ng13 and rxjs7 and found my tests were subscribing to the obs outside of a fakeAsync (in my beforeEach). I was able to resolve this by moving the subscription (literally the .subscribe call) into the individual test's body, wrapped in a fakeAsync.

Maybe unrelated to other comments, but wanted to share one more thing to look for.

[artaommahe]

@JoostK can someone look at this please? we have a reproduction with clean ng app above

[JoostK]

@artaommahe The EMPTY.pipe(delay(100)) in that test used to behave differently in RxJS 6: it used to delay the completion signal of EMPTY by the provided delay but this is not the case in RxJS 7: the completion signal from EMPTY immediately completes the Observable, resulting in broken rate limiting.

[artaommahe]

@JoostK oh, thx. there is nothing about these changes here https://rxjs.dev/deprecations/breaking-changes 😕

[lionel-meunier]

+1

[KupnoH]

+1. Having the same issue with angular 13.2.2

[JiaLiPassion]

@artaommahe, just like @JoostK mentioned, the EMPTY.pipe(delay(100)) seems to be the change of rxjs, so it is not an issue of zone or angular testing.
And about how to put the logic of beforeEach and test body (such as it()) into the same fakeAsync() context, there is a working PR to address this issue now #40611.

[svasile8]

+1

[ToelliJ]

@JoostK oh, thx. there is nothing about these changes here https://rxjs.dev/deprecations/breaking-changes 😕

You can find a notice on 6-to-7-change-summary.

I had the same issue. I found out that you can fix the tests by changing the implementation from in example debounceTime(500) to debounce(() => interval(500)). Generally I do not like to change the real implementation only to fix the unit tests but in this case I did it. In my understanding debounce() is similar to debounceTime() and the only difference is, that another observable will provide the time span.

[ma7moudat]

Is there a plan to solve this or should we learn to live with it? Or should we consider it an issue with rxjs instead of Angular? 🤔

I agree with @ToelliJ; it doesn't make sense to revise the component code to make the test pass :|

[philmayfield]

I also fixed this recently by moving my suite wide beforeEach logic into a callable function, and for the tests that use fakeAsync calling that function in each it block. The tests that didn't need fakeAsync could all be called in a beforeEach. Annoying - but it is what it is?

[avast-ye]

This worked for me...

import { interval, of } from 'rxjs';
import * as rxjsOperators from 'rxjs/operators';
import { debounce } from 'rxjs/operators';

// in test suit setup...
jest.spyOn(rxjsOperators, 'debounceTime').mockImplementation((timeout) => debounce(() => interval(timeout)))

[BaronVonPerko]

Moving the code that subscribes to the observable within the fakeAsync fixed it for me. A bit of a pain since I have tests with this setup in the beginning of my suites, but it works.

[pfeileon]

// component
@Component({
  selector: 'app-test',
  template: '<p *ngIf="test$ | async">test works!</p>\n',
  styleUrls: ['./test.component.scss'],
  standalone: true,
  imports: [CommonModule]
})
export class TestComponent {
  // doesn't matter if it's timer (one-off) or interval
  readonly test$ = interval(3000).pipe(
    map(() => 'test'),
    tap(console.log),
  );
}

// spec
  // this doesn't work
  beforeEach(async () => {
    await TestBed.compileComponents();

    fixture = TestBed.createComponent(TestComponent);
    fixture.detectChanges();
  });

  it('should work', fakeAsync(() => {
    tick(4000);
    fixture.detectChanges();
    expect(fixture.nativeElement.querySelector('p')).toBeTruthy();
    discardPeriodicTasks();
  }));


  // this works
  it('should work', fakeAsync(async () => {
    await TestBed.compileComponents();

    fixture = TestBed.createComponent(TestComponent);
    fixture.detectChanges();

    tick(3000);
    fixture.detectChanges();
    expect(fixture.nativeElement.querySelector('p')).toBeTruthy();
    discardPeriodicTasks();
  }));

Angular 15.1.0

[lswl66]

ReactiveX/rxjs#6382 see rxjs this issue

[ananthay]

The workaround suggested by @bgerstle-quot worked for me as well (thank you!). But this was such an annoying issue as it does not happen for all the components.
+1 for angular to fix the bug.

[bdugan14]

This solution works for me. In my case debounceTime(500) with input value change. Instead of fakeAsync and tick I am using waitForAsync and fixture.whenStable.

it('should work with waitForAsync', waitForAsync(() => {
    spyOn(component, 'handleFormChanges').and.callThrough();

    const nameInput = fixture.debugElement.queryAll(By.css('input'))[0];
    nameInput.nativeElement.value = 'John';
    nameInput.nativeElement.dispatchEvent(new Event('input'));

    fixture.whenStable().then(() => {
      expect(component.handleFormChanges).toHaveBeenCalled();
    });
  }));

@pkozlowski-opensource we tried this strategy - but it wasn't correctly reporting failures for us: whatever expectations we put in the whenStable().then(.. callback would always pass (for instance, expect(true).toEqual(false)). Does it correctly report failed expectations for you?

[matheo]

@ananthay I don't see this issue depending on Angular but on RxJs team.
Our tests are broken and we're using jest...