From b7fd86ec50245b67aa8ec2f217ec67f64d7576b9 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Wed, 20 Nov 2019 17:10:15 +0200 Subject: [PATCH] test(docs-infra): destroy all `ScrollService` instances after each test (#33937) `ScrollService` subscribes to global `window` events and mutates global state in the listener (e.g. read/write values from/to `sessionStorage`). Therefore, we need to always call its `ngOnDestroy()` method to unsubscribe from these events after each test. In f69c6e204, a new testcase was introduced that was not destroyed. As a result, random failures started to randomly happen in other, unrelated tests ([example CI failure][1]). This commit fixes this by ensuring all `ScrollService` instances are destroyed after each tests (provided that they are created with the `createScrollService()` helper). [1]: https://circleci.com/gh/angular/angular/533298 PR Close #33937 --- aio/src/app/shared/scroll.service.spec.ts | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/aio/src/app/shared/scroll.service.spec.ts b/aio/src/app/shared/scroll.service.spec.ts index 99b405fc1a..5e2e4c4de9 100644 --- a/aio/src/app/shared/scroll.service.spec.ts +++ b/aio/src/app/shared/scroll.service.spec.ts @@ -7,6 +7,13 @@ import { fakeAsync, tick } from '@angular/core/testing'; import { ScrollService, topMargin } from './scroll.service'; describe('ScrollService', () => { + const scrollServiceInstances: ScrollService[] = []; + const createScrollService = (...args: ConstructorParameters) => { + const instance = new ScrollService(...args); + scrollServiceInstances.push(instance); + return instance; + }; + const topOfPageElem = {} as Element; let injector: ReflectiveInjector; let document: MockDocument; @@ -36,7 +43,11 @@ describe('ScrollService', () => { beforeEach(() => { injector = ReflectiveInjector.resolveAndCreate([ - ScrollService, + { + provide: ScrollService, + useFactory: createScrollService, + deps: [DOCUMENT, PlatformLocation, ViewportScroller, Location], + }, { provide: Location, useClass: SpyLocation }, { provide: DOCUMENT, useClass: MockDocument }, { provide: PlatformLocation, useClass: MockPlatformLocation }, @@ -51,7 +62,7 @@ describe('ScrollService', () => { spyOn(window, 'scrollBy'); }); - afterEach(() => scrollService.ngOnDestroy()); + afterEach(() => scrollServiceInstances.forEach(instance => instance.ngOnDestroy())); it('should debounce `updateScrollPositonInHistory()`', fakeAsync(() => { const updateScrollPositionInHistorySpy = spyOn(scrollService, 'updateScrollPositionInHistory'); @@ -100,7 +111,7 @@ describe('ScrollService', () => { expect(() => { const platformLoc = platformLocation as PlatformLocation; - const service = new ScrollService(document, platformLoc, viewportScrollerStub, location); + const service = createScrollService(document, platformLoc, viewportScrollerStub, location); service.updateScrollLocationHref(); expect(service.getStoredScrollLocationHref()).toBeNull();