diff --git a/aio/e2e/app.e2e-spec.ts b/aio/e2e/app.e2e-spec.ts index c60c6022b9..15e4ad9f71 100644 --- a/aio/e2e/app.e2e-spec.ts +++ b/aio/e2e/app.e2e-spec.ts @@ -68,30 +68,33 @@ describe('site App', function() { // TODO(https://github.com/angular/angular/issues/19785): Activate this again // once it is no more flaky. - xdescribe('google analytics', () => { - beforeEach(done => page.gaReady.then(done)); - - it('should call ga', done => { - page.ga() - .then(calls => { - expect(calls.length).toBeGreaterThan(2, 'ga calls'); - done(); - }); - }); + describe('google analytics', () => { it('should call ga with initial URL', done => { let path: string; - + page.navigateTo('api'); page.locationPath() .then(p => path = p) .then(() => page.ga().then(calls => { - expect(calls.length).toBeGreaterThan(2, 'ga calls'); - expect(calls[1]).toEqual(['set', 'page', path]); + // The last call (length-1) will be the `send` command + // The second to last call (length-2) will be the command to `set` the page url + expect(calls[calls.length - 2]).toEqual(['set', 'page', path]); done(); })); }); - // Todo: add test to confirm tracking URL when navigate. + it('should call ga with new URL on navigation', done => { + let path: string; + page.getLink('features').click(); + page.locationPath() + .then(p => path = p) + .then(() => page.ga().then(calls => { + // The last call (length-1) will be the `send` command + // The second to last call (length-2) will be the command to `set` the page url + expect(calls[calls.length - 2]).toEqual(['set', 'page', path]); + done(); + })); + }); }); describe('search', () => { diff --git a/aio/e2e/app.po.ts b/aio/e2e/app.po.ts index cf71d51d61..8e1d906e1b 100644 --- a/aio/e2e/app.po.ts +++ b/aio/e2e/app.po.ts @@ -12,7 +12,6 @@ export class SitePage { .all(by.css('a')) .filter((a: ElementFinder) => a.getAttribute('href').then(href => githubRegex.test(href))) .first(); - gaReady: promise.Promise; static setWindowWidth(newWidth: number) { const win = browser.driver.manage().window(); @@ -25,11 +24,14 @@ export class SitePage { .first(); } getLink(path) { return element(by.css(`a[href="${path}"]`)); } - ga() { return browser.executeScript('return window["gaCalls"]') as promise.Promise; } + ga() { return browser.executeScript('return window["ga"].q') as promise.Promise; } locationPath() { return browser.executeScript('return document.location.pathname') as promise.Promise; } navigateTo(pageUrl = '') { - return browser.get('/' + pageUrl).then(_ => this.replaceGa(_)); + return browser.get('/' + pageUrl) + // We need to tell the index.html not to load the real analytics library + // See the GA snippet in index.html + .then(() => browser.driver.executeScript('sessionStorage.setItem("__e2e__", true);')); } getDocViewerText() { @@ -61,31 +63,4 @@ export class SitePage { browser.wait(ExpectedConditions.presenceOf(results.first()), 8000); return results; } - - /** - * Replace the ambient Google Analytics tracker with homebrew spy - * don't send commands to GA during e2e testing! - * @param _ - forward's anything passed in - */ - private replaceGa(_: any) { - - this.gaReady = browser.driver.executeScript(() => { - // Give ga() a "ready" callback: - // https://developers.google.com/analytics/devguides/collection/analyticsjs/command-queue-reference - window['ga'](() => { - window['gaCalls'] = []; - window['ga'] = function() { window['gaCalls'].push(arguments); }; - }); - - }) - .then(() => { - // wait for GaService to start using window.ga after analytics lib loads. - const d = promise.defer(); - setTimeout(() => d.fulfill(), 1000); // GaService.initializeDelay - return d.promise; - }); - - return _; - } } - diff --git a/aio/src/app/app.module.ts b/aio/src/app/app.module.ts index 52815c6511..671ef5f10c 100644 --- a/aio/src/app/app.module.ts +++ b/aio/src/app/app.module.ts @@ -45,6 +45,7 @@ import { ScrollService } from 'app/shared/scroll.service'; import { ScrollSpyService } from 'app/shared/scroll-spy.service'; import { SearchBoxComponent } from './search/search-box/search-box.component'; import { TocService } from 'app/shared/toc.service'; +import { WindowToken, windowProvider } from 'app/shared/window'; import { SharedModule } from 'app/shared/shared.module'; @@ -113,7 +114,8 @@ export const svgIconProviders = [ ScrollSpyService, SearchService, svgIconProviders, - TocService + TocService, + { provide: WindowToken, useFactory: windowProvider }, ], bootstrap: [AppComponent] }) diff --git a/aio/src/app/shared/ga.service.spec.ts b/aio/src/app/shared/ga.service.spec.ts index 5281142ced..be3c80cc59 100644 --- a/aio/src/app/shared/ga.service.spec.ts +++ b/aio/src/app/shared/ga.service.spec.ts @@ -1,130 +1,87 @@ import { ReflectiveInjector } from '@angular/core'; -import { fakeAsync, tick } from '@angular/core/testing'; import { GaService } from 'app/shared/ga.service'; -import { Logger } from 'app/shared/logger.service'; +import { WindowToken } from 'app/shared/window'; describe('GaService', () => { - let gaSpy: jasmine.Spy; + let gaService: GaService; let injector: ReflectiveInjector; - - // filter for 'send' which communicates with server - // returns the url of the 'send pageview' - function gaSpySendCalls() { - let lastUrl: string; - return gaSpy.calls.all() - .reduce((acc, c) => { - const args = c.args; - if (args[0] === 'set') { - lastUrl = args[2]; - } else if (args[0] === 'send') { - acc.push(lastUrl); - } - return acc; - }, []); - } + let gaSpy: jasmine.Spy; + let mockWindow: any; beforeEach(() => { - injector = ReflectiveInjector.resolveAndCreate([ - GaService, - { provide: Logger, useClass: TestLogger } - ]); + gaSpy = jasmine.createSpy('ga'); + mockWindow = { ga: gaSpy }; + injector = ReflectiveInjector.resolveAndCreate([GaService, { provide: WindowToken, useFactory: () => mockWindow }]); + gaService = injector.get(GaService); }); - describe('with ambient GA', () => { - let gaService: GaService; - - beforeEach(fakeAsync(() => { - this.winGa = window['ga']; // remember current GA tracker just in case - - // Replace Google Analytics tracker with spy after calling "ga ready" callback - window['ga'] = (fn: Function) => { - window['ga'] = gaSpy = jasmine.createSpy('ga'); - fn(); - tick(GaService.initializeDelay); // see GaService#initializeGa - }; - gaService = injector.get(GaService); - })); - - afterEach(() => { - window['ga'] = this.winGa; - }); - - it('should initialize ga with "create" when constructed', () => { - const first = gaSpy.calls.first().args; - expect(first[0]).toBe('create'); - }); - - describe('#locationChanged(url)', () => { - it('should send page to url w/ leading slash', () => { - gaService.locationChanged('testUrl'); - let args = gaSpy.calls.all()[1].args; - expect(args).toEqual(['set', 'page', '/testUrl']); - args = gaSpy.calls.all()[2].args; - expect(args).toEqual(['send', 'pageview']); - }); - }); - - describe('#sendPage(url)', () => { - it('should set page to url w/ leading slash', () => { - gaService.sendPage('testUrl'); - const args = gaSpy.calls.all()[1].args; - expect(args).toEqual(['set', 'page', '/testUrl']); - }); - - it('should send "pageview" ', () => { - gaService.sendPage('testUrl'); - const args = gaSpy.calls.all()[2].args; - expect(args).toEqual(['send', 'pageview']); - }); - - it('should not send twice with same URL, back-to-back', () => { - gaService.sendPage('testUrl'); - gaService.sendPage('testUrl'); - expect(gaSpySendCalls()).toEqual(['/testUrl']); - }); - - it('should send twice with same URL, back-to-back, even when the hash changes', () => { - // Therefore it is up to caller NOT to call it when hash changes if this is unwanted. - // See LocationService and its specs - gaService.sendPage('testUrl#one'); - gaService.sendPage('testUrl#two'); - expect(gaSpySendCalls()).toEqual([ - '/testUrl#one', - '/testUrl#two' - ]); - - }); - - it('should send same URL twice when other intervening URL', () => { - gaService.sendPage('testUrl'); - gaService.sendPage('testUrl2'); - gaService.sendPage('testUrl'); - expect(gaSpySendCalls()).toEqual([ - '/testUrl', - '/testUrl2', - '/testUrl' - ]); - }); - }); - + it('should initialize ga with "create" when constructed', () => { + const first = gaSpy.calls.first().args; + expect(first[0]).toBe('create'); }); - describe('when no ambient GA', () => { - let gaService: GaService; - let logger: TestLogger; - - it('should log with "create" when constructed', () => { - gaService = injector.get(GaService); - logger = injector.get(Logger); - expect(logger.log.calls.count()).toBe(1, 'logger.log should be called'); - const first = logger.log.calls.first().args; - expect(first[0]).toBe('ga:'); - expect(first[1][0]).toBe('create'); // first[1] is the array of args to ga() + describe('#locationChanged(url)', () => { + it('should send page to url w/ leading slash', () => { + gaService.locationChanged('testUrl'); + expect(gaSpy).toHaveBeenCalledWith('set', 'page', '/testUrl'); + expect(gaSpy).toHaveBeenCalledWith('send', 'pageview'); }); }); + + describe('#sendPage(url)', () => { + it('should set page to url w/ leading slash', () => { + gaService.sendPage('testUrl'); + expect(gaSpy).toHaveBeenCalledWith('set', 'page', '/testUrl'); + }); + + it('should send "pageview" ', () => { + gaService.sendPage('testUrl'); + expect(gaSpy).toHaveBeenCalledWith('send', 'pageview'); + }); + + it('should not send twice with same URL, back-to-back', () => { + gaService.sendPage('testUrl'); + gaSpy.calls.reset(); + gaService.sendPage('testUrl'); + expect(gaSpy).not.toHaveBeenCalled(); + }); + + it('should send again even if only the hash changes', () => { + // Therefore it is up to caller NOT to call it when hash changes if this is unwanted. + // See LocationService and its specs + gaService.sendPage('testUrl#one'); + expect(gaSpy).toHaveBeenCalledWith('set', 'page', '/testUrl#one'); + expect(gaSpy).toHaveBeenCalledWith('send', 'pageview'); + gaSpy.calls.reset(); + gaService.sendPage('testUrl#two'); + expect(gaSpy).toHaveBeenCalledWith('set', 'page', '/testUrl#two'); + expect(gaSpy).toHaveBeenCalledWith('send', 'pageview'); + }); + + it('should send same URL twice when other intervening URL', () => { + gaService.sendPage('testUrl'); + expect(gaSpy).toHaveBeenCalledWith('set', 'page', '/testUrl'); + expect(gaSpy).toHaveBeenCalledWith('send', 'pageview'); + gaSpy.calls.reset(); + gaService.sendPage('testUrl2'); + expect(gaSpy).toHaveBeenCalledWith('set', 'page', '/testUrl2'); + expect(gaSpy).toHaveBeenCalledWith('send', 'pageview'); + gaSpy.calls.reset(); + gaService.sendPage('testUrl'); + expect(gaSpy).toHaveBeenCalledWith('set', 'page', '/testUrl'); + expect(gaSpy).toHaveBeenCalledWith('send', 'pageview'); + }); + }); + + it('should support replacing the `window.ga` function', () => { + const gaSpy2 = jasmine.createSpy('new ga'); + mockWindow.ga = gaSpy2; + gaSpy.calls.reset(); + + gaService.sendPage('testUrl'); + expect(gaSpy).not.toHaveBeenCalled(); + expect(gaSpy2).toHaveBeenCalledWith('set', 'page', '/testUrl'); + expect(gaSpy2).toHaveBeenCalledWith('send', 'pageview'); + }); }); - -class TestLogger { - log = jasmine.createSpy('log'); -} diff --git a/aio/src/app/shared/ga.service.ts b/aio/src/app/shared/ga.service.ts index c9563f9cf8..747e8285b4 100644 --- a/aio/src/app/shared/ga.service.ts +++ b/aio/src/app/shared/ga.service.ts @@ -1,7 +1,7 @@ -import { Injectable } from '@angular/core'; +import { Inject, Injectable } from '@angular/core'; import { environment } from '../../environments/environment'; -import { Logger } from 'app/shared/logger.service'; +import { WindowToken } from 'app/shared/window'; @Injectable() /** @@ -10,15 +10,10 @@ import { Logger } from 'app/shared/logger.service'; * Associates data with a GA "property" from the environment (`gaId`). */ export class GaService { - // ms to wait before acquiring window.ga after analytics library loads - // empirically determined to allow time for e2e test setup - static initializeDelay = 1000; private previousUrl: string; - private ga: (...rest: any[]) => void; - constructor(private logger: Logger) { - this.initializeGa(); + constructor(@Inject(WindowToken) private window: Window) { this.ga('create', environment['gaId'] , 'auto'); } @@ -34,27 +29,7 @@ export class GaService { this.ga('send', 'pageview'); } - // These gyrations are necessary to make the service e2e testable - // and to disable ga tracking during e2e tests. - private initializeGa() { - const ga = window['ga']; - if (ga) { - // Queue commands until GA analytics script has loaded. - const gaQueue: any[][] = []; - this.ga = (...rest: any[]) => { gaQueue.push(rest); }; - - // Then send queued commands to either real or e2e test ga(); - // after waiting to allow possible e2e test to replace global ga function - ga(() => setTimeout(() => { - // this.logger.log('GA fn:', window['ga'].toString()); - this.ga = window['ga']; - gaQueue.forEach((command) => this.ga.apply(null, command)); - }, GaService.initializeDelay)); - - } else { - // delegate `ga` calls to the logger if no ga installed - this.ga = (...rest: any[]) => { this.logger.log('ga:', rest); }; - } + ga(...args) { + this.window['ga'](...args); } - } diff --git a/aio/src/app/shared/window.ts b/aio/src/app/shared/window.ts new file mode 100644 index 0000000000..c1eba5a7e8 --- /dev/null +++ b/aio/src/app/shared/window.ts @@ -0,0 +1,4 @@ +import { InjectionToken } from '@angular/core'; + +export const WindowToken = new InjectionToken('Window'); +export function windowProvider() { return window; } diff --git a/aio/src/index.html b/aio/src/index.html index fca6077a28..9318d4af05 100644 --- a/aio/src/index.html +++ b/aio/src/index.html @@ -34,9 +34,13 @@ -