From 112dff81b032810097f46d0690a89aa7b195d6ec Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Tue, 22 Dec 2020 12:50:51 -0800 Subject: [PATCH] test(router): update scroller tests to use real objects (#40241) The current tests in the router scroller are [change-detector tests](https://testing.googleblog.com/2015/01/testing-on-toilet-change-detector-tests.html) and do not ensure the correct behavior of the scroller. This commit updates the tests to assert actual scrolling behavior of the browser. PR Close #40241 --- .../common/test/viewport_scroller_spec.ts | 77 +++++++++++-------- 1 file changed, 44 insertions(+), 33 deletions(-) diff --git a/packages/common/test/viewport_scroller_spec.ts b/packages/common/test/viewport_scroller_spec.ts index 11e5d7b9a1..58ffb5bb1d 100644 --- a/packages/common/test/viewport_scroller_spec.ts +++ b/packages/common/test/viewport_scroller_spec.ts @@ -10,19 +10,17 @@ import {describe, expect, it} from '@angular/core/testing/src/testing_internal'; import {BrowserViewportScroller, ViewportScroller} from '../src/viewport_scroller'; describe('BrowserViewportScroller', () => { - let scroller: ViewportScroller; - let documentSpy: any; - let windowSpy: any; - - beforeEach(() => { - windowSpy = - jasmine.createSpyObj('window', ['history', 'scrollTo', 'pageXOffset', 'pageYOffset']); - windowSpy.history.scrollRestoration = 'auto'; - documentSpy = jasmine.createSpyObj('document', ['getElementById', 'getElementsByName']); - scroller = new BrowserViewportScroller(documentSpy, windowSpy); - }); - describe('setHistoryScrollRestoration', () => { + let scroller: ViewportScroller; + let windowSpy: any; + + beforeEach(() => { + windowSpy = + jasmine.createSpyObj('window', ['history', 'scrollTo', 'pageXOffset', 'pageYOffset']); + windowSpy.history.scrollRestoration = 'auto'; + scroller = new BrowserViewportScroller(document, windowSpy); + }); + function createNonWritableScrollRestoration() { Object.defineProperty(windowSpy.history, 'scrollRestoration', { value: 'auto', @@ -43,34 +41,47 @@ describe('BrowserViewportScroller', () => { }); describe('scrollToAnchor', () => { + // Testing scroll behavior does not make sense outside a browser + if (isNode) return; const anchor = 'anchor'; - const el = document.createElement('a'); + let tallItem: HTMLDivElement; + let el: HTMLAnchorElement; + let scroller: BrowserViewportScroller; - it('should only call getElementById when an element is found by id', () => { - documentSpy.getElementById.and.returnValue(el); - spyOn(scroller, 'scrollToElement'); - scroller.scrollToAnchor(anchor); - expect(documentSpy.getElementById).toHaveBeenCalledWith(anchor); - expect(documentSpy.getElementsByName).not.toHaveBeenCalled(); - expect((scroller as any).scrollToElement).toHaveBeenCalledWith(el); + beforeEach(() => { + scroller = new BrowserViewportScroller(document, window); + scroller.scrollToPosition([0, 0]); + + tallItem = document.createElement('div'); + tallItem.style.height = '3000px'; + document.body.appendChild(tallItem); + + el = document.createElement('a'); + el.innerText = 'some link'; + el.href = '#'; + document.body.appendChild(el); }); - it('should call getElementById and getElementsByName when an element is found by name', () => { - documentSpy.getElementsByName.and.returnValue([el]); - spyOn(scroller, 'scrollToElement'); - scroller.scrollToAnchor(anchor); - expect(documentSpy.getElementById).toHaveBeenCalledWith(anchor); - expect(documentSpy.getElementsByName).toHaveBeenCalledWith(anchor); - expect((scroller as any).scrollToElement).toHaveBeenCalledWith(el); + afterEach(() => { + document.body.removeChild(tallItem); + document.body.removeChild(el); }); - it('should not call scrollToElement when an element is not found by its id or its name', () => { - documentSpy.getElementsByName.and.returnValue([]); - spyOn(scroller, 'scrollToElement'); + it('should scroll when element with matching id is found', () => { + el.id = anchor; scroller.scrollToAnchor(anchor); - expect(documentSpy.getElementById).toHaveBeenCalledWith(anchor); - expect(documentSpy.getElementsByName).toHaveBeenCalledWith(anchor); - expect((scroller as any).scrollToElement).not.toHaveBeenCalled(); + expect(scroller.getScrollPosition()[1]).not.toEqual(0); + }); + + it('should scroll when anchor with matching name is found', () => { + el.name = anchor; + scroller.scrollToAnchor(anchor); + expect(scroller.getScrollPosition()[1]).not.toEqual(0); + }); + + it('should not scroll when no matching element is found', () => { + scroller.scrollToAnchor(anchor); + expect(scroller.getScrollPosition()[1]).toEqual(0); }); }); });