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

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

About this issue

  • Original URL
  • State: open
  • Created 3 years ago
  • Reactions: 47
  • Comments: 23 (7 by maintainers)

Most upvoted comments

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.

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

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.

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 😐

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

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

@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

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?