honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(aio): simplify GaService to avoid e2e test failures 

The GaService and the E2E specs were unnecessarily complicated and had
arbitrary async timeouts to ensure that the interplay between the GA
library code and the rest of the app worked correctly. This resulted
in potential flaky tests if the timeouts were not adequate; this was
experienced when Travis upgraded to Chrome 62.

The new approach is to block loading of the Analytics library altogether
if there is a `__e2e__` flag set in the `SessionStorage` of the browser.
It doesn't appear to be enough just to set the flag directly on the
window. I think this is because the window gets cleaned when navigation
occurs (but I am not certain).

The downside of this is that we had to add a small piece of extra logic
to the GA snippet in index.html; and we also had to switch from using
`<script async ...>` to a programmatic approach to loading the GA library
which prevents the browser from eagerly fetching the library. This may
be mitigated by adding it to the HTTP/2 push configuration of the Firebase
hosting.

Re-enables the test that was disabled in https://github.com/angular/angular/pull/19784

Closes #19785
This commit is contained in:
Peter Bacon Darwin 2017-10-19 13:58:27 +01:00 committed by Alex Rickabaugh
parent 14380ff086
commit 441e01c568
7 changed files with 112 additions and 192 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
aio/e2e/app.e2e-spec.ts
View File

@ -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', () => {

35
aio/e2e/app.po.ts
View File

@ -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<any>;
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<any[][]>; }
ga() { return browser.executeScript('return window["ga"].q') as promise.Promise<any[][]>; }
locationPath() { return browser.executeScript('return document.location.pathname') as promise.Promise<string>; }
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 _;
}
}

4
aio/src/app/app.module.ts
View File

@ -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]
})

113
aio/src/app/shared/ga.service.spec.ts
View File

@ -1,53 +1,19 @@
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 }
]);
});
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
};
gaSpy = jasmine.createSpy('ga');
mockWindow = { ga: gaSpy };
injector = ReflectiveInjector.resolveAndCreate([GaService, { provide: WindowToken, useFactory: () => mockWindow }]);
gaService = injector.get(GaService);
}));
afterEach(() => {
window['ga'] = this.winGa;
});
it('should initialize ga with "create" when constructed', () => {
@ -58,73 +24,64 @@ describe('GaService', () => {
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']);
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');
const args = gaSpy.calls.all()[1].args;
expect(args).toEqual(['set', 'page', '/testUrl']);
expect(gaSpy).toHaveBeenCalledWith('set', 'page', '/testUrl');
});
it('should send "pageview" ', () => {
gaService.sendPage('testUrl');
const args = gaSpy.calls.all()[2].args;
expect(args).toEqual(['send', 'pageview']);
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(gaSpySendCalls()).toEqual(['/testUrl']);
expect(gaSpy).not.toHaveBeenCalled();
});
it('should send twice with same URL, back-to-back, even when the hash changes', () => {
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(gaSpySendCalls()).toEqual([
'/testUrl#one',
'/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(gaSpySendCalls()).toEqual([
'/testUrl',
'/testUrl2',
'/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();
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()
gaService.sendPage('testUrl');
expect(gaSpy).not.toHaveBeenCalled();
expect(gaSpy2).toHaveBeenCalledWith('set', 'page', '/testUrl');
expect(gaSpy2).toHaveBeenCalledWith('send', 'pageview');
});
});
});
class TestLogger {
log = jasmine.createSpy('log');
}

35
aio/src/app/shared/ga.service.ts
View File

@ -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);
}
}
}

4
aio/src/app/shared/window.ts Normal file
View File

@ -0,0 +1,4 @@
import { InjectionToken } from '@angular/core';
export const WindowToken = new InjectionToken('Window');
export function windowProvider() { return window; }

8
aio/src/index.html
View File

@ -34,9 +34,13 @@
<!-- Google Analytics -->
<script>
window.ga=window.ga||function(){(ga.q=ga.q||[]).push(arguments)};ga.l=+new Date;
// Note this is a customised version of the GA tracking snippet to aid e2e testing
// See the bit between /**/.../**/
(function(i,s,o,g,r,a,m){i['GoogleAnalyticsObject']=r;i[r]=i[r]||function(){
(i[r].q=i[r].q||[]).push(arguments)},i[r].l=1*new Date();a=s.createElement(o),
m=s.getElementsByTagName(o)[0];a.async=1;a.src=g;/**/i.sessionStorage.__e2e__||/**/m.parentNode.insertBefore(a,m)
})(window,document,'script','https://www.google-analytics.com/analytics.js','ga');
</script>
<script async src='https://www.google-analytics.com/analytics.js'></script>
<!-- End Google Analytics -->
<script>