From b3a87fb4a905652a097ab21a29326ca5f5dea5ae Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Fri, 23 Feb 2018 01:44:35 +0200 Subject: [PATCH 01/10] fix(aio): wait for the app to stabilize before registering the SW This commit also waits for the app to stabilize, before starting to check for ServiceWorker updates. This avoids setting up a long timeout, which would prevent the app from stabilizing and thus cause issues with Protractor. --- .../app/sw-updates/sw-updates.service.spec.ts | 31 +++++++++++++++---- aio/src/app/sw-updates/sw-updates.service.ts | 20 ++++++------ aio/src/main.ts | 2 +- 3 files changed, 36 insertions(+), 17 deletions(-) diff --git a/aio/src/app/sw-updates/sw-updates.service.spec.ts b/aio/src/app/sw-updates/sw-updates.service.spec.ts index d32eb23524b0b..12a48436ea705 100644 --- a/aio/src/app/sw-updates/sw-updates.service.spec.ts +++ b/aio/src/app/sw-updates/sw-updates.service.spec.ts @@ -1,4 +1,4 @@ -import { ReflectiveInjector } from '@angular/core'; +import { ApplicationRef, ReflectiveInjector } from '@angular/core'; import { fakeAsync, tick } from '@angular/core/testing'; import { NgServiceWorker } from '@angular/service-worker'; import { Subject } from 'rxjs'; @@ -9,23 +9,26 @@ import { SwUpdatesService } from './sw-updates.service'; describe('SwUpdatesService', () => { let injector: ReflectiveInjector; + let appRef: MockApplicationRef; let service: SwUpdatesService; let sw: MockNgServiceWorker; let checkInterval: number; // Helpers // NOTE: - // Because `SwUpdatesService` uses the `debounceTime` operator, it needs to be instantiated - // inside the `fakeAsync` zone (when `fakeAsync` is used for the test). Thus, we can't run - // `setup()` in a `beforeEach()` block. We use the `run()` helper to call `setup()` inside each - // test's zone. + // Because `SwUpdatesService` uses the `debounceTime` operator, it needs to be instantiated and + // destroyed inside the `fakeAsync` zone (when `fakeAsync` is used for the test). Thus, we can't + // run `setup()`/`tearDown()` in `beforeEach()`/`afterEach()` blocks. We use the `run()` helper + // to call them inside each test's zone. const setup = () => { injector = ReflectiveInjector.resolveAndCreate([ + { provide: ApplicationRef, useClass: MockApplicationRef }, { provide: Logger, useClass: MockLogger }, { provide: NgServiceWorker, useClass: MockNgServiceWorker }, SwUpdatesService ]); + appRef = injector.get(ApplicationRef); service = injector.get(SwUpdatesService); sw = injector.get(NgServiceWorker); checkInterval = (service as any).checkInterval; @@ -42,11 +45,18 @@ describe('SwUpdatesService', () => { expect(service).toBeTruthy(); })); - it('should immediately check for updates when instantiated', run(() => { + it('should start checking for updates when instantiated (once the app stabilizes)', run(() => { + expect(sw.checkForUpdate).not.toHaveBeenCalled(); + + appRef.isStable.next(false); + expect(sw.checkForUpdate).not.toHaveBeenCalled(); + + appRef.isStable.next(true); expect(sw.checkForUpdate).toHaveBeenCalled(); })); it('should schedule a new check if there is no update available', fakeAsync(run(() => { + appRef.isStable.next(true); sw.checkForUpdate.calls.reset(); sw.$$checkForUpdateSubj.next(false); @@ -58,6 +68,7 @@ describe('SwUpdatesService', () => { }))); it('should activate new updates immediately', fakeAsync(run(() => { + appRef.isStable.next(true); sw.checkForUpdate.calls.reset(); sw.$$checkForUpdateSubj.next(true); @@ -69,6 +80,7 @@ describe('SwUpdatesService', () => { }))); it('should not pass a specific version to `NgServiceWorker.activateUpdate()`', fakeAsync(run(() => { + appRef.isStable.next(true); sw.$$checkForUpdateSubj.next(true); tick(checkInterval); @@ -76,6 +88,7 @@ describe('SwUpdatesService', () => { }))); it('should schedule a new check after activating the update', fakeAsync(run(() => { + appRef.isStable.next(true); sw.checkForUpdate.calls.reset(); sw.$$checkForUpdateSubj.next(true); @@ -103,6 +116,7 @@ describe('SwUpdatesService', () => { describe('when destroyed', () => { it('should not schedule a new check for update (after current check)', fakeAsync(run(() => { + appRef.isStable.next(true); sw.checkForUpdate.calls.reset(); service.ngOnDestroy(); @@ -113,6 +127,7 @@ describe('SwUpdatesService', () => { }))); it('should not schedule a new check for update (after activating an update)', fakeAsync(run(() => { + appRef.isStable.next(true); sw.checkForUpdate.calls.reset(); sw.$$checkForUpdateSubj.next(true); @@ -141,6 +156,10 @@ describe('SwUpdatesService', () => { }); // Mocks +class MockApplicationRef { + isStable = new Subject(); +} + class MockLogger { log = jasmine.createSpy('MockLogger.log'); } diff --git a/aio/src/app/sw-updates/sw-updates.service.ts b/aio/src/app/sw-updates/sw-updates.service.ts index cbc373aeef60e..cc9e1d6721074 100644 --- a/aio/src/app/sw-updates/sw-updates.service.ts +++ b/aio/src/app/sw-updates/sw-updates.service.ts @@ -1,7 +1,8 @@ -import { Injectable, OnDestroy } from '@angular/core'; +import { ApplicationRef, Injectable, OnDestroy } from '@angular/core'; import { NgServiceWorker } from '@angular/service-worker'; import { concat, of, Subject } from 'rxjs'; -import { debounceTime, filter, map, startWith, take, takeUntil, tap } from 'rxjs/operators'; +import { debounceTime, filter, first, map, startWith, takeUntil, tap } from 'rxjs/operators'; + import { Logger } from 'app/shared/logger.service'; @@ -29,13 +30,12 @@ export class SwUpdatesService implements OnDestroy { map(({version}) => version), ); - constructor(private logger: Logger, private sw: NgServiceWorker) { - this.checkForUpdateSubj - .pipe( - debounceTime(this.checkInterval), - startWith(undefined), - takeUntil(this.onDestroy), - ) + constructor(appRef: ApplicationRef, private logger: Logger, private sw: NgServiceWorker) { + const appIsStable$ = appRef.isStable.pipe(filter(v => v), first()); + const checkForUpdates$ = this.checkForUpdateSubj.pipe(debounceTime(this.checkInterval), startWith(undefined)); + + concat(appIsStable$, checkForUpdates$) + .pipe(takeUntil(this.onDestroy)) .subscribe(() => this.checkForUpdate()); } @@ -55,7 +55,7 @@ export class SwUpdatesService implements OnDestroy { // TODO (gkalpak): Remove once #137 is fixed. concat(this.sw.checkForUpdate(), of(false)) .pipe( - take(1), + first(), tap(v => this.log(`Update available: ${v}`)), ) .subscribe(v => v ? this.activateUpdate() : this.scheduleCheckForUpdate()); diff --git a/aio/src/main.ts b/aio/src/main.ts index de675719f7a74..2d2f9568880e1 100644 --- a/aio/src/main.ts +++ b/aio/src/main.ts @@ -12,7 +12,7 @@ if (environment.production) { platformBrowserDynamic().bootstrapModule(AppModule).then(ref => { if (environment.production && 'serviceWorker' in (navigator as any)) { const appRef: ApplicationRef = ref.injector.get(ApplicationRef); - appRef.isStable.pipe(first()).subscribe(() => { + appRef.isStable.pipe(first(v => v)).subscribe(() => { (navigator as any).serviceWorker.register('/worker-basic.min.js'); }); } From cc5ee5f1d90cafd4aacea955fa493ae57e9d5700 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Wed, 28 Feb 2018 17:24:21 +0200 Subject: [PATCH 02/10] fixup! fix(aio): wait for the app to stabilize before registering the SW --- aio/src/app/sw-updates/sw-updates.service.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/aio/src/app/sw-updates/sw-updates.service.ts b/aio/src/app/sw-updates/sw-updates.service.ts index cc9e1d6721074..6c0aef47d50cf 100644 --- a/aio/src/app/sw-updates/sw-updates.service.ts +++ b/aio/src/app/sw-updates/sw-updates.service.ts @@ -1,7 +1,7 @@ import { ApplicationRef, Injectable, OnDestroy } from '@angular/core'; import { NgServiceWorker } from '@angular/service-worker'; import { concat, of, Subject } from 'rxjs'; -import { debounceTime, filter, first, map, startWith, takeUntil, tap } from 'rxjs/operators'; +import { debounceTime, defaultIfEmpty, filter, first, map, startWith, takeUntil, tap } from 'rxjs/operators'; import { Logger } from 'app/shared/logger.service'; @@ -31,7 +31,7 @@ export class SwUpdatesService implements OnDestroy { ); constructor(appRef: ApplicationRef, private logger: Logger, private sw: NgServiceWorker) { - const appIsStable$ = appRef.isStable.pipe(filter(v => v), first()); + const appIsStable$ = appRef.isStable.pipe(first(v => v)); const checkForUpdates$ = this.checkForUpdateSubj.pipe(debounceTime(this.checkInterval), startWith(undefined)); concat(appIsStable$, checkForUpdates$) @@ -51,10 +51,11 @@ export class SwUpdatesService implements OnDestroy { private checkForUpdate() { this.log('Checking for update...'); - // Temp workaround for https://github.com/angular/mobile-toolkit/pull/137. - // TODO (gkalpak): Remove once #137 is fixed. - concat(this.sw.checkForUpdate(), of(false)) + this.sw.checkForUpdate() .pipe( + // Temp workaround for https://github.com/angular/mobile-toolkit/pull/137. + // TODO (gkalpak): Remove once #137 is fixed. + defaultIfEmpty(false), first(), tap(v => this.log(`Update available: ${v}`)), ) From cdc2e0945f19077d65f67b602b7db9e191af0241 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Wed, 28 Feb 2018 01:14:28 +0200 Subject: [PATCH 03/10] refactor(aio): move deployment config tests and helpers around This commit prepares the ground for adding different types of tests. --- aio/package.json | 2 +- .../shared}/URLS_TO_REDIRECT.txt | 0 .../shared}/helpers.ts | 23 +++++++++++-------- .../unit}/testFirebaseRedirection.spec.ts | 4 ++-- .../unit}/testServiceWorkerRoutes.spec.ts | 4 ++-- 5 files changed, 18 insertions(+), 15 deletions(-) rename aio/tests/{deployment => deployment-config/shared}/URLS_TO_REDIRECT.txt (100%) rename aio/tests/{deployment => deployment-config/shared}/helpers.ts (66%) rename aio/tests/{deployment => deployment-config/unit}/testFirebaseRedirection.spec.ts (85%) rename aio/tests/{deployment => deployment-config/unit}/testServiceWorkerRoutes.spec.ts (90%) diff --git a/aio/package.json b/aio/package.json index 28449ffa01e53..612a5bb892f66 100644 --- a/aio/package.json +++ b/aio/package.json @@ -44,7 +44,7 @@ "docs-watch": "node tools/transforms/authors-package/watchr.js", "docs-lint": "eslint --ignore-path=\"tools/transforms/.eslintignore\" tools/transforms", "docs-test": "node tools/transforms/test.js", - "deployment-config-test": "jasmine-ts tests/deployment/**/*.spec.ts", + "deployment-config-test": "jasmine-ts tests/deployment-config/unit/**/*.spec.ts", "firebase-utils-test": "jasmine-ts tools/firebase-test-utils/*.spec.ts", "tools-lint": "tslint -c \"tools/tslint.json\" \"tools/firebase-test-utils/**/*.ts\"", "tools-test": "./scripts/deploy-to-firebase.test.sh && yarn docs-test && yarn boilerplate:test && jasmine tools/ng-packages-installer/index.spec.js && yarn firebase-utils-test", diff --git a/aio/tests/deployment/URLS_TO_REDIRECT.txt b/aio/tests/deployment-config/shared/URLS_TO_REDIRECT.txt similarity index 100% rename from aio/tests/deployment/URLS_TO_REDIRECT.txt rename to aio/tests/deployment-config/shared/URLS_TO_REDIRECT.txt diff --git a/aio/tests/deployment/helpers.ts b/aio/tests/deployment-config/shared/helpers.ts similarity index 66% rename from aio/tests/deployment/helpers.ts rename to aio/tests/deployment-config/shared/helpers.ts index 0347597f285d4..e4a28d3074d2e 100644 --- a/aio/tests/deployment/helpers.ts +++ b/aio/tests/deployment-config/shared/helpers.ts @@ -1,21 +1,24 @@ -const { readFileSync } = require('fs'); -const path = require('canonical-path'); -const cjson = require('cjson'); +import { resolve } from 'canonical-path'; +import { load as loadJson } from 'cjson'; +import { readFileSync } from 'fs'; -import { FirebaseRedirector, FirebaseRedirectConfig } from '../../tools/firebase-test-utils/FirebaseRedirector'; +import { FirebaseRedirector, FirebaseRedirectConfig } from '../../../tools/firebase-test-utils/FirebaseRedirector'; + + +const AIO_DIR = resolve(__dirname, '../../..'); export function getRedirector() { return new FirebaseRedirector(loadRedirects()); } export function loadRedirects(): FirebaseRedirectConfig[] { - const pathToFirebaseJSON = path.resolve(__dirname, '../../firebase.json'); - const contents = cjson.load(pathToFirebaseJSON); + const pathToFirebaseJSON = `${AIO_DIR}/firebase.json`; + const contents = loadJson(pathToFirebaseJSON); return contents.hosting.redirects; } export function loadSitemapUrls() { - const pathToSiteMap = path.resolve(__dirname, '../../src/generated/sitemap.xml'); + const pathToSiteMap = `${AIO_DIR}/src/generated/sitemap.xml`; const xml = readFileSync(pathToSiteMap, 'utf8'); const urls: string[] = []; xml.replace(/([^<]+)<\/loc>/g, (_, loc) => urls.push(loc.replace('%%DEPLOYMENT_HOST%%', ''))); @@ -23,14 +26,14 @@ export function loadSitemapUrls() { } export function loadLegacyUrls() { - const pathToLegacyUrls = path.resolve(__dirname, 'URLS_TO_REDIRECT.txt'); + const pathToLegacyUrls = `${__dirname}/URLS_TO_REDIRECT.txt`; const urls = readFileSync(pathToLegacyUrls, 'utf8').split('\n').map(line => line.split('\t')); return urls; } export function loadSWRoutes() { - const pathToSWManifest = path.resolve(__dirname, '../../ngsw-manifest.json'); - const contents = cjson.load(pathToSWManifest); + const pathToSWManifest = `${AIO_DIR}/ngsw-manifest.json`; + const contents = loadJson(pathToSWManifest); const routes = contents.routing.routes; return Object.keys(routes).map(route => { const routeConfig = routes[route]; diff --git a/aio/tests/deployment/testFirebaseRedirection.spec.ts b/aio/tests/deployment-config/unit/testFirebaseRedirection.spec.ts similarity index 85% rename from aio/tests/deployment/testFirebaseRedirection.spec.ts rename to aio/tests/deployment-config/unit/testFirebaseRedirection.spec.ts index 19b687456d242..ca047d78ef17c 100644 --- a/aio/tests/deployment/testFirebaseRedirection.spec.ts +++ b/aio/tests/deployment-config/unit/testFirebaseRedirection.spec.ts @@ -1,8 +1,8 @@ -import { getRedirector, loadLegacyUrls, loadRedirects, loadSitemapUrls } from './helpers'; +import { getRedirector, loadLegacyUrls, loadRedirects, loadLocalSitemapUrls } from '../shared/helpers'; describe('firebase.json redirect config', () => { describe('with sitemap urls', () => { - loadSitemapUrls().forEach(url => { + loadLocalSitemapUrls().forEach(url => { it('should not redirect any urls in the sitemap', () => { expect(getRedirector().redirect(url)).toEqual(url); }); diff --git a/aio/tests/deployment/testServiceWorkerRoutes.spec.ts b/aio/tests/deployment-config/unit/testServiceWorkerRoutes.spec.ts similarity index 90% rename from aio/tests/deployment/testServiceWorkerRoutes.spec.ts rename to aio/tests/deployment-config/unit/testServiceWorkerRoutes.spec.ts index db119ffecb932..95cec6143d882 100644 --- a/aio/tests/deployment/testServiceWorkerRoutes.spec.ts +++ b/aio/tests/deployment-config/unit/testServiceWorkerRoutes.spec.ts @@ -1,8 +1,8 @@ -import { loadLegacyUrls, loadSitemapUrls, loadSWRoutes } from './helpers'; +import { loadLegacyUrls, loadLocalSitemapUrls, loadSWRoutes } from '../shared/helpers'; describe('service-worker routes', () => { - loadSitemapUrls().forEach(url => { + loadLocalSitemapUrls().forEach(url => { it('should process URLs in the Sitemap', () => { const routes = loadSWRoutes(); expect(routes.some(test => test(url))).toBeTruthy(url); From a353b4be4d883c51cbafeabc30f4e5fc622ef999 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Wed, 28 Feb 2018 17:38:34 +0200 Subject: [PATCH 04/10] fixup! refactor(aio): move deployment config tests and helpers around --- .../deployment-config/unit/testFirebaseRedirection.spec.ts | 4 ++-- .../deployment-config/unit/testServiceWorkerRoutes.spec.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/aio/tests/deployment-config/unit/testFirebaseRedirection.spec.ts b/aio/tests/deployment-config/unit/testFirebaseRedirection.spec.ts index ca047d78ef17c..b9a018dfea9a3 100644 --- a/aio/tests/deployment-config/unit/testFirebaseRedirection.spec.ts +++ b/aio/tests/deployment-config/unit/testFirebaseRedirection.spec.ts @@ -1,8 +1,8 @@ -import { getRedirector, loadLegacyUrls, loadRedirects, loadLocalSitemapUrls } from '../shared/helpers'; +import { getRedirector, loadLegacyUrls, loadRedirects, loadSitemapUrls } from '../shared/helpers'; describe('firebase.json redirect config', () => { describe('with sitemap urls', () => { - loadLocalSitemapUrls().forEach(url => { + loadSitemapUrls().forEach(url => { it('should not redirect any urls in the sitemap', () => { expect(getRedirector().redirect(url)).toEqual(url); }); diff --git a/aio/tests/deployment-config/unit/testServiceWorkerRoutes.spec.ts b/aio/tests/deployment-config/unit/testServiceWorkerRoutes.spec.ts index 95cec6143d882..ad115b1fd2135 100644 --- a/aio/tests/deployment-config/unit/testServiceWorkerRoutes.spec.ts +++ b/aio/tests/deployment-config/unit/testServiceWorkerRoutes.spec.ts @@ -1,8 +1,8 @@ -import { loadLegacyUrls, loadLocalSitemapUrls, loadSWRoutes } from '../shared/helpers'; +import { loadLegacyUrls, loadSitemapUrls, loadSWRoutes } from '../shared/helpers'; describe('service-worker routes', () => { - loadLocalSitemapUrls().forEach(url => { + loadSitemapUrls().forEach(url => { it('should process URLs in the Sitemap', () => { const routes = loadSWRoutes(); expect(routes.some(test => test(url))).toBeTruthy(url); From 2fc4428fd7ffb66d9065d7da20eca9818031b61c Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Wed, 28 Feb 2018 01:16:58 +0200 Subject: [PATCH 05/10] fix(aio): fix SW routing RegExp to allow redirecting `/api/animate` URLs --- aio/ngsw-manifest.json | 2 +- aio/tests/deployment-config/shared/URLS_TO_REDIRECT.txt | 9 ++++++++- aio/tests/deployment-config/shared/helpers.ts | 7 +++++-- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/aio/ngsw-manifest.json b/aio/ngsw-manifest.json index 35584cc0676ba..0be0c68de37bc 100644 --- a/aio/ngsw-manifest.json +++ b/aio/ngsw-manifest.json @@ -19,7 +19,7 @@ "routing": { "index": "/index.html", "routes": { - "^(?!/styleguide|/docs/.|(?:/guide/(?:cli-quickstart|metadata|ngmodule|service-worker-(?:getstart|comm|configref)|learning-angular)|/news)(?:\\.html|/)?$|/testing|/api/(?:.+/[^/]+-|platform-browser/AnimationDriver|testing|api/|(?:common/(?:NgModel|Control|MaxLengthValidator))|(?:[^/]+/)?(?:NgFor(?:$|-)|AnimationStateDeclarationMetadata|CORE_DIRECTIVES|PLATFORM_PIPES|DirectiveMetadata|HTTP_PROVIDERS))|.*/stackblitz(?:\\.html)?$|.*\\.[^\/.]+$)": { + "^(?!/styleguide|/docs/.|(?:/guide/(?:cli-quickstart|metadata|ngmodule|service-worker-(?:getstart|comm|configref)|learning-angular)|/news)(?:\\.html|/)?$|/testing|/api/(?:.+/[^/]+-|platform-browser/AnimationDriver|testing/|api/|animate/|(?:common/(?:NgModel|Control|MaxLengthValidator))|(?:[^/]+/)?(?:NgFor(?:$|-)|AnimationStateDeclarationMetadata|CORE_DIRECTIVES|PLATFORM_PIPES|DirectiveMetadata|HTTP_PROVIDERS))|.*/stackblitz(?:\\.html)?$|.*\\.[^\/.]+$)": { "match": "regex" } } diff --git a/aio/tests/deployment-config/shared/URLS_TO_REDIRECT.txt b/aio/tests/deployment-config/shared/URLS_TO_REDIRECT.txt index cdc6575d20098..7d715a1ca83c0 100644 --- a/aio/tests/deployment-config/shared/URLS_TO_REDIRECT.txt +++ b/aio/tests/deployment-config/shared/URLS_TO_REDIRECT.txt @@ -1,3 +1,5 @@ +# Note: Empty lines and lines starting with `#` are ignored. + /api/api/core/ElementRef /api/core/ElementRef /api/common/Control-class /api/forms/FormControl /api/common/CORE_DIRECTIVES /api/common/CommonModule @@ -179,4 +181,9 @@ /news https://blog.angular.io/ /news.html https://blog.angular.io/ /testing /guide/testing -/testing/first-app-tests.html /guide/testing \ No newline at end of file +/testing/first-app-tests.html /guide/testing + +# These URLs are deliberately put at the end in order to ensure that the ServiceWorker will have +# been installed. This allows verifying that the SW will indeed let them pass through to the server. +/api/animate/AnimationBuilder /api/animations/AnimationBuilder +/api/animate/CssAnimationBuilder /api/animations/CssAnimationBuilder diff --git a/aio/tests/deployment-config/shared/helpers.ts b/aio/tests/deployment-config/shared/helpers.ts index e4a28d3074d2e..465d066459552 100644 --- a/aio/tests/deployment-config/shared/helpers.ts +++ b/aio/tests/deployment-config/shared/helpers.ts @@ -27,8 +27,11 @@ export function loadSitemapUrls() { export function loadLegacyUrls() { const pathToLegacyUrls = `${__dirname}/URLS_TO_REDIRECT.txt`; - const urls = readFileSync(pathToLegacyUrls, 'utf8').split('\n').map(line => line.split('\t')); - return urls; + return readFileSync(pathToLegacyUrls, 'utf8') + .split('\n') // Split text into lines. + .map(line => line.trim()) // Trim leading/trailing whitespace. + .filter(line => line && !/^#/.test(line)) // Remove empty lines and comments. + .map(line => line.split('\t')); // Split lines into URL pairs. } export function loadSWRoutes() { From 873284c12cb68e0efbeddbe21f2cf1427cf35b01 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Wed, 28 Feb 2018 21:06:54 +0200 Subject: [PATCH 06/10] fixup! fix(aio): fix SW routing RegExp to allow redirecting `/api/animate` URLs --- .../deployment-config/shared/URLS_TO_REDIRECT.txt | 11 +++-------- aio/tests/deployment-config/shared/helpers.ts | 7 ++----- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/aio/tests/deployment-config/shared/URLS_TO_REDIRECT.txt b/aio/tests/deployment-config/shared/URLS_TO_REDIRECT.txt index 7d715a1ca83c0..c4d9ee2d4f491 100644 --- a/aio/tests/deployment-config/shared/URLS_TO_REDIRECT.txt +++ b/aio/tests/deployment-config/shared/URLS_TO_REDIRECT.txt @@ -1,5 +1,5 @@ -# Note: Empty lines and lines starting with `#` are ignored. - +/api/animate/AnimationBuilder /api/animations/AnimationBuilder +/api/animate/CssAnimationBuilder /api/animations/CssAnimationBuilder /api/api/core/ElementRef /api/core/ElementRef /api/common/Control-class /api/forms/FormControl /api/common/CORE_DIRECTIVES /api/common/CommonModule @@ -181,9 +181,4 @@ /news https://blog.angular.io/ /news.html https://blog.angular.io/ /testing /guide/testing -/testing/first-app-tests.html /guide/testing - -# These URLs are deliberately put at the end in order to ensure that the ServiceWorker will have -# been installed. This allows verifying that the SW will indeed let them pass through to the server. -/api/animate/AnimationBuilder /api/animations/AnimationBuilder -/api/animate/CssAnimationBuilder /api/animations/CssAnimationBuilder +/testing/first-app-tests.html /guide/testing \ No newline at end of file diff --git a/aio/tests/deployment-config/shared/helpers.ts b/aio/tests/deployment-config/shared/helpers.ts index 465d066459552..e4a28d3074d2e 100644 --- a/aio/tests/deployment-config/shared/helpers.ts +++ b/aio/tests/deployment-config/shared/helpers.ts @@ -27,11 +27,8 @@ export function loadSitemapUrls() { export function loadLegacyUrls() { const pathToLegacyUrls = `${__dirname}/URLS_TO_REDIRECT.txt`; - return readFileSync(pathToLegacyUrls, 'utf8') - .split('\n') // Split text into lines. - .map(line => line.trim()) // Trim leading/trailing whitespace. - .filter(line => line && !/^#/.test(line)) // Remove empty lines and comments. - .map(line => line.split('\t')); // Split lines into URL pairs. + const urls = readFileSync(pathToLegacyUrls, 'utf8').split('\n').map(line => line.split('\t')); + return urls; } export function loadSWRoutes() { From 014ce820e152e22caa5625ed34619c56d7b16e8a Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Wed, 28 Feb 2018 01:24:07 +0200 Subject: [PATCH 07/10] ci(aio): add monitoring for angular.io This commit configures a periodic job to be run on CircleCI, performing several checks against the actual apps deployed to production (https://angular.io) and staging (https://next.angular.io). Fixes #21942 --- .circleci/config.yml | 19 ++++++++ aio/package.json | 10 ++-- aio/scripts/test-production.sh | 38 +++++++++++++++ .../deployment-config/e2e/protractor.conf.js | 46 +++++++++++++++++++ .../e2e/testDeployedRedirection.e2e-spec.ts | 35 ++++++++++++++ aio/tests/deployment-config/shared/helpers.ts | 38 +++++++++++---- 6 files changed, 173 insertions(+), 13 deletions(-) create mode 100644 aio/scripts/test-production.sh create mode 100644 aio/tests/deployment-config/e2e/protractor.conf.js create mode 100644 aio/tests/deployment-config/e2e/testDeployedRedirection.e2e-spec.ts diff --git a/.circleci/config.yml b/.circleci/config.yml index ddba460813758..7d4978db2a48c 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -98,9 +98,28 @@ jobs: - "node_modules" - "~/bazel_repository_cache" + aio_monitoring: + <<: *job_defaults + steps: + - checkout: + <<: *post_checkout + - restore_cache: + key: *cache_key + - run: xvfb-run --auto-servernum ./aio/scripts/test-production.sh + workflows: version: 2 default_workflow: jobs: - lint - build + aio_monitoring: + jobs: + - aio_monitoring + triggers: + - schedule: + cron: "0 0 * * *" + filters: + branches: + only: + - master diff --git a/aio/package.json b/aio/package.json index 612a5bb892f66..f3e2809ce5b68 100644 --- a/aio/package.json +++ b/aio/package.json @@ -6,6 +6,8 @@ "author": "Angular", "license": "MIT", "scripts": { + "preinstall": "node ../tools/yarn/check-yarn.js", + "postinstall": "node tools/cli-patches/patch.js && uglifyjs node_modules/lunr/lunr.js -c -m -o src/assets/js/lunr.min.js --source-map", "aio-use-local": "node tools/ng-packages-installer overwrite . --debug --ignore-packages @angular/service-worker", "aio-use-npm": "node tools/ng-packages-installer restore .", "aio-check-local": "node tools/ng-packages-installer check .", @@ -17,10 +19,9 @@ "build-local": "yarn ~~build", "lint": "yarn check-env && yarn docs-lint && ng lint && yarn example-lint && yarn tools-lint", "test": "yarn check-env && ng test", - "pree2e": "yarn check-env && yarn ~~update-webdriver", + "pree2e": "yarn check-env && yarn update-webdriver", "e2e": "ng e2e --no-webdriver-update", "e2e-prod": "yarn e2e --environment=dev --target=production", - "preinstall": "node ../tools/yarn/check-yarn.js", "presetup": "yarn install --frozen-lockfile && yarn ~~check-env && yarn boilerplate:remove", "setup": "yarn aio-use-npm && yarn example-use-npm", "postsetup": "yarn boilerplate:add && yarn build-ie-polyfills && yarn docs", @@ -57,12 +58,11 @@ "generate-zips": "node ./tools/example-zipper/generateZips", "sw-manifest": "ngu-sw-manifest --dist dist --in ngsw-manifest.json --out dist/ngsw-manifest.json", "sw-copy": "cp node_modules/@angular/service-worker/bundles/worker-basic.min.js dist/", - "postinstall": "node tools/cli-patches/patch.js && uglifyjs node_modules/lunr/lunr.js -c -m -o src/assets/js/lunr.min.js --source-map", "build-ie-polyfills": "node node_modules/webpack/bin/webpack.js -p src/ie-polyfills.js src/generated/ie-polyfills.min.js", + "update-webdriver": "webdriver-manager update --standalone false --gecko false $CHROMEDRIVER_VERSION_ARG", "~~check-env": "node scripts/check-environment", "~~build": "ng build --target=production --environment=stable -sm", - "post~~build": "yarn sw-manifest && yarn sw-copy", - "~~update-webdriver": "webdriver-manager update --standalone false --gecko false $CHROMEDRIVER_VERSION_ARG" + "post~~build": "yarn sw-manifest && yarn sw-copy" }, "engines": { "node": ">=8.9.1 <9.0.0", diff --git a/aio/scripts/test-production.sh b/aio/scripts/test-production.sh new file mode 100644 index 0000000000000..71988b136a638 --- /dev/null +++ b/aio/scripts/test-production.sh @@ -0,0 +1,38 @@ +#!/usr/bin/env bash +set +x -eu -o pipefail + +( + readonly thisDir="$(cd $(dirname ${BASH_SOURCE[0]}); pwd)" + readonly aioDir="$(realpath $thisDir/..)" + + readonly appPtorConf="$aioDir/tests/e2e/protractor.conf.js" + readonly cfgPtorConf="$aioDir/tests/deployment-config/e2e/protractor.conf.js" + readonly minPwaScore="95" + readonly urls=( + "https://angular.io/" + "https://next.angular.io" + ) + + cd "$aioDir" + + # Install dependencies. + echo -e "\nInstalling dependencies in '$aioDir'...\n-----" + yarn install --frozen-lockfile + yarn update-webdriver + + # Run checks for all URLs. + for url in "${urls[@]}"; do + echo -e "\nChecking '$url'...\n-----" + + # Run e2e tests. + yarn protractor "$appPtorConf" --baseUrl "$url" + + # Run deployment config tests. + yarn protractor "$cfgPtorConf" --baseUrl "$url" + + # Run PWA-score tests. + yarn test-pwa-score "$url" "$minPwaScore" + done + + echo -e "\nAll checks passed!" +) diff --git a/aio/tests/deployment-config/e2e/protractor.conf.js b/aio/tests/deployment-config/e2e/protractor.conf.js new file mode 100644 index 0000000000000..4c066d77e87d4 --- /dev/null +++ b/aio/tests/deployment-config/e2e/protractor.conf.js @@ -0,0 +1,46 @@ +// Protractor configuration file, see link for more information +// https://github.com/angular/protractor/blob/master/lib/config.ts + +exports.config = { + allScriptsTimeout: 11000, + specs: [ + './*.e2e-spec.ts' + ], + capabilities: { + browserName: 'chrome', + // For Travis + chromeOptions: { + binary: process.env.CHROME_BIN, + args: ['--no-sandbox'] + } + }, + directConnect: true, + framework: 'jasmine', + jasmineNodeOpts: { + showColors: true, + defaultTimeoutInterval: 30000, + print: function() {} + }, + params: { + sitemapUrls: [], + legacyUrls: [], + }, + beforeLaunch() { + const {register} = require('ts-node'); + register({}); + }, + onPrepare() { + const {SpecReporter} = require('jasmine-spec-reporter'); + const {browser} = require('protractor'); + const {loadLegacyUrls, loadRemoteSitemapUrls} = require('../shared/helpers'); + + return Promise.all([ + browser.getProcessedConfig(), + loadRemoteSitemapUrls(browser.baseUrl), + loadLegacyUrls(), + ]).then(([config, sitemapUrls, legacyUrls]) => { + Object.assign(config.params, {sitemapUrls, legacyUrls}); + jasmine.getEnv().addReporter(new SpecReporter({spec: {displayStacktrace: true}})); + }); + } +}; diff --git a/aio/tests/deployment-config/e2e/testDeployedRedirection.e2e-spec.ts b/aio/tests/deployment-config/e2e/testDeployedRedirection.e2e-spec.ts new file mode 100644 index 0000000000000..ba2285789ab99 --- /dev/null +++ b/aio/tests/deployment-config/e2e/testDeployedRedirection.e2e-spec.ts @@ -0,0 +1,35 @@ +import { browser } from 'protractor'; + +describe(browser.baseUrl, () => { + const sitemapUrls = browser.params.sitemapUrls; + const legacyUrls = browser.params.legacyUrls; + + beforeEach(() => browser.waitForAngularEnabled(false)); + afterEach(() => browser.waitForAngularEnabled(true)); + + describe('(with sitemap URLs)', () => { + sitemapUrls.forEach((url, i) => { + it(`should not redirect '${url}' (${i + 1}/${sitemapUrls.length})`, async () => { + browser.get(url); + + const expectedUrl = browser.baseUrl + url; + const actualUrl = (await browser.getCurrentUrl()).replace(/\?.*$/, ''); + + expect(actualUrl).toBe(expectedUrl); + }); + }); + }); + + describe('(with legacy URLs)', () => { + legacyUrls.forEach(([fromUrl, toUrl], i) => { + it(`should redirect '${fromUrl}' to '${toUrl}' (${i + 1}/${legacyUrls.length})`, async () => { + browser.get(fromUrl); + + const expectedUrl = (/^http/.test(toUrl) ? '' : browser.baseUrl.replace(/\/$/, '')) + toUrl; + const actualUrl = (await browser.getCurrentUrl()).replace(/\?.*$/, ''); + + expect(actualUrl).toBe(expectedUrl); + }); + }); + }); +}); diff --git a/aio/tests/deployment-config/shared/helpers.ts b/aio/tests/deployment-config/shared/helpers.ts index e4a28d3074d2e..7a3ff1651a217 100644 --- a/aio/tests/deployment-config/shared/helpers.ts +++ b/aio/tests/deployment-config/shared/helpers.ts @@ -1,6 +1,8 @@ import { resolve } from 'canonical-path'; import { load as loadJson } from 'cjson'; import { readFileSync } from 'fs'; +import { get as httpGet } from 'http'; +import { get as httpsGet } from 'https'; import { FirebaseRedirector, FirebaseRedirectConfig } from '../../../tools/firebase-test-utils/FirebaseRedirector'; @@ -17,20 +19,33 @@ export function loadRedirects(): FirebaseRedirectConfig[] { return contents.hosting.redirects; } -export function loadSitemapUrls() { - const pathToSiteMap = `${AIO_DIR}/src/generated/sitemap.xml`; - const xml = readFileSync(pathToSiteMap, 'utf8'); - const urls: string[] = []; - xml.replace(/([^<]+)<\/loc>/g, (_, loc) => urls.push(loc.replace('%%DEPLOYMENT_HOST%%', ''))); - return urls; -} - export function loadLegacyUrls() { const pathToLegacyUrls = `${__dirname}/URLS_TO_REDIRECT.txt`; const urls = readFileSync(pathToLegacyUrls, 'utf8').split('\n').map(line => line.split('\t')); return urls; } +export function loadLocalSitemapUrls() { + const pathToSiteMap = `${AIO_DIR}/src/generated/sitemap.xml`; + const xml = readFileSync(pathToSiteMap, 'utf8'); + return extractSitemapUrls(xml); +} + +export async function loadRemoteSitemapUrls(host: string) { + const urlToSiteMap = `${host}/generated/sitemap.xml`; + const get = /^https:/.test(host) ? httpsGet : httpGet; + + const xml = await new Promise((resolve, reject) => { + let responseText = ''; + get(urlToSiteMap, res => res + .on('data', chunk => responseText += chunk) + .on('end', () => resolve(responseText)) + .on('error', reject)); + }); + + return extractSitemapUrls(xml, host); +} + export function loadSWRoutes() { const pathToSWManifest = `${AIO_DIR}/ngsw-manifest.json`; const contents = loadJson(pathToSWManifest); @@ -50,3 +65,10 @@ export function loadSWRoutes() { } }); } + +// Private functions +function extractSitemapUrls(xml: string, host = '%%DEPLOYMENT_HOST%%') { + const urls: string[] = []; + xml.replace(/([^<]+)<\/loc>/g, (_, loc) => urls.push(loc.replace(host, '')) as any); + return urls; +} From dc97fd6e8f855d2fc13898fcd96d4d663a6dd7bf Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Wed, 28 Feb 2018 17:40:58 +0200 Subject: [PATCH 08/10] fixup! ci(aio): add monitoring for angular.io --- aio/tests/deployment-config/e2e/protractor.conf.js | 6 ++++++ .../e2e/testDeployedRedirection.e2e-spec.ts | 13 ++++++++++++- .../unit/testFirebaseRedirection.spec.ts | 4 ++-- .../unit/testServiceWorkerRoutes.spec.ts | 4 ++-- 4 files changed, 22 insertions(+), 5 deletions(-) diff --git a/aio/tests/deployment-config/e2e/protractor.conf.js b/aio/tests/deployment-config/e2e/protractor.conf.js index 4c066d77e87d4..72a1894efbb2b 100644 --- a/aio/tests/deployment-config/e2e/protractor.conf.js +++ b/aio/tests/deployment-config/e2e/protractor.conf.js @@ -39,6 +39,12 @@ exports.config = { loadRemoteSitemapUrls(browser.baseUrl), loadLegacyUrls(), ]).then(([config, sitemapUrls, legacyUrls]) => { + if (sitemapUrls.length <= 100) { + throw new Error(`Too few sitemap URLs. (Expected: >100 | Found: ${sitemapUrls.length})`); + } else if (legacyUrls.length <= 100) { + throw new Error(`Too few legacy URLs. (Expected: >100 | Found: ${legacyUrls.length})`); + } + Object.assign(config.params, {sitemapUrls, legacyUrls}); jasmine.getEnv().addReporter(new SpecReporter({spec: {displayStacktrace: true}})); }); diff --git a/aio/tests/deployment-config/e2e/testDeployedRedirection.e2e-spec.ts b/aio/tests/deployment-config/e2e/testDeployedRedirection.e2e-spec.ts index ba2285789ab99..2ad4e6fd7a464 100644 --- a/aio/tests/deployment-config/e2e/testDeployedRedirection.e2e-spec.ts +++ b/aio/tests/deployment-config/e2e/testDeployedRedirection.e2e-spec.ts @@ -4,7 +4,18 @@ describe(browser.baseUrl, () => { const sitemapUrls = browser.params.sitemapUrls; const legacyUrls = browser.params.legacyUrls; - beforeEach(() => browser.waitForAngularEnabled(false)); + beforeEach(async (done) => { + // Unregister the ServiceWorker to ensure requests are passed through to the server. + await browser.get(browser.baseUrl); + await browser.executeAsyncScript(cb => navigator.serviceWorker + .getRegistrations() + .then(regs => Promise.all(regs.map(reg => reg.unregister()))) + .then(cb)); + + browser.waitForAngularEnabled(false); + done(); + }); + afterEach(() => browser.waitForAngularEnabled(true)); describe('(with sitemap URLs)', () => { diff --git a/aio/tests/deployment-config/unit/testFirebaseRedirection.spec.ts b/aio/tests/deployment-config/unit/testFirebaseRedirection.spec.ts index b9a018dfea9a3..d50f81f32db77 100644 --- a/aio/tests/deployment-config/unit/testFirebaseRedirection.spec.ts +++ b/aio/tests/deployment-config/unit/testFirebaseRedirection.spec.ts @@ -1,8 +1,8 @@ -import { getRedirector, loadLegacyUrls, loadRedirects, loadSitemapUrls } from '../shared/helpers'; +import { getRedirector, loadLegacyUrls, loadLocalSitemapUrls, loadRedirects } from '../shared/helpers'; describe('firebase.json redirect config', () => { describe('with sitemap urls', () => { - loadSitemapUrls().forEach(url => { + loadLocalSitemapUrls().forEach(url => { it('should not redirect any urls in the sitemap', () => { expect(getRedirector().redirect(url)).toEqual(url); }); diff --git a/aio/tests/deployment-config/unit/testServiceWorkerRoutes.spec.ts b/aio/tests/deployment-config/unit/testServiceWorkerRoutes.spec.ts index ad115b1fd2135..95cec6143d882 100644 --- a/aio/tests/deployment-config/unit/testServiceWorkerRoutes.spec.ts +++ b/aio/tests/deployment-config/unit/testServiceWorkerRoutes.spec.ts @@ -1,8 +1,8 @@ -import { loadLegacyUrls, loadSitemapUrls, loadSWRoutes } from '../shared/helpers'; +import { loadLegacyUrls, loadLocalSitemapUrls, loadSWRoutes } from '../shared/helpers'; describe('service-worker routes', () => { - loadSitemapUrls().forEach(url => { + loadLocalSitemapUrls().forEach(url => { it('should process URLs in the Sitemap', () => { const routes = loadSWRoutes(); expect(routes.some(test => test(url))).toBeTruthy(url); From b4036638e11229da2e64afb009c368258cc42a69 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Mon, 5 Mar 2018 14:36:17 +0200 Subject: [PATCH 09/10] fixup! ci(aio): add monitoring for angular.io --- aio/.gitignore | 3 ++- .../e2e/testDeployedRedirection.e2e-spec.ts | 18 +++++++++++------- aio/tests/deployment-config/shared/helpers.ts | 4 +++- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/aio/.gitignore b/aio/.gitignore index 4ac6ca5434eb5..4790f4f2f739f 100644 --- a/aio/.gitignore +++ b/aio/.gitignore @@ -30,6 +30,7 @@ /connect.lock /coverage /libpeerconnection.log +debug.log npm-debug.log testem.log /typings @@ -45,4 +46,4 @@ protractor-results*.txt Thumbs.db # copied dependencies -src/assets/js/lunr* \ No newline at end of file +src/assets/js/lunr* diff --git a/aio/tests/deployment-config/e2e/testDeployedRedirection.e2e-spec.ts b/aio/tests/deployment-config/e2e/testDeployedRedirection.e2e-spec.ts index 2ad4e6fd7a464..9c7b8d41d0e1f 100644 --- a/aio/tests/deployment-config/e2e/testDeployedRedirection.e2e-spec.ts +++ b/aio/tests/deployment-config/e2e/testDeployedRedirection.e2e-spec.ts @@ -3,25 +3,29 @@ import { browser } from 'protractor'; describe(browser.baseUrl, () => { const sitemapUrls = browser.params.sitemapUrls; const legacyUrls = browser.params.legacyUrls; - - beforeEach(async (done) => { - // Unregister the ServiceWorker to ensure requests are passed through to the server. - await browser.get(browser.baseUrl); + const goTo = async url => { + // Go to the specified URL and then unregister the ServiceWorker + // to ensure subsequent requests are passed through to the server. + await browser.get(url); await browser.executeAsyncScript(cb => navigator.serviceWorker .getRegistrations() .then(regs => Promise.all(regs.map(reg => reg.unregister()))) .then(cb)); + }; - browser.waitForAngularEnabled(false); + beforeAll(async done => { + // Make an initial request to unregister the ServiceWorker. + await goTo(browser.baseUrl); done(); }); + beforeEach(() => browser.waitForAngularEnabled(false)); afterEach(() => browser.waitForAngularEnabled(true)); describe('(with sitemap URLs)', () => { sitemapUrls.forEach((url, i) => { it(`should not redirect '${url}' (${i + 1}/${sitemapUrls.length})`, async () => { - browser.get(url); + await goTo(url); const expectedUrl = browser.baseUrl + url; const actualUrl = (await browser.getCurrentUrl()).replace(/\?.*$/, ''); @@ -34,7 +38,7 @@ describe(browser.baseUrl, () => { describe('(with legacy URLs)', () => { legacyUrls.forEach(([fromUrl, toUrl], i) => { it(`should redirect '${fromUrl}' to '${toUrl}' (${i + 1}/${legacyUrls.length})`, async () => { - browser.get(fromUrl); + await goTo(fromUrl); const expectedUrl = (/^http/.test(toUrl) ? '' : browser.baseUrl.replace(/\/$/, '')) + toUrl; const actualUrl = (await browser.getCurrentUrl()).replace(/\?.*$/, ''); diff --git a/aio/tests/deployment-config/shared/helpers.ts b/aio/tests/deployment-config/shared/helpers.ts index 7a3ff1651a217..d50a6ae375a4f 100644 --- a/aio/tests/deployment-config/shared/helpers.ts +++ b/aio/tests/deployment-config/shared/helpers.ts @@ -43,7 +43,9 @@ export async function loadRemoteSitemapUrls(host: string) { .on('error', reject)); }); - return extractSitemapUrls(xml, host); + // Currently, all sitemaps use `angular.io` as host in URLs (which is fine since we only use the + // sitemap `angular.io`). See also `aio/src/extra-files/*/robots.txt`. + return extractSitemapUrls(xml, 'https://angular.io/'); } export function loadSWRoutes() { From a3000e2dc4699aaca827ec5159adf54aa1b51928 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Mon, 26 Mar 2018 13:56:20 +0300 Subject: [PATCH 10/10] fixup! fix(aio): wait for the app to stabilize before registering the SW --- aio/src/app/sw-updates/sw-updates.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aio/src/app/sw-updates/sw-updates.service.ts b/aio/src/app/sw-updates/sw-updates.service.ts index 6c0aef47d50cf..e858990eddf06 100644 --- a/aio/src/app/sw-updates/sw-updates.service.ts +++ b/aio/src/app/sw-updates/sw-updates.service.ts @@ -1,6 +1,6 @@ import { ApplicationRef, Injectable, OnDestroy } from '@angular/core'; import { NgServiceWorker } from '@angular/service-worker'; -import { concat, of, Subject } from 'rxjs'; +import { concat, Subject } from 'rxjs'; import { debounceTime, defaultIfEmpty, filter, first, map, startWith, takeUntil, tap } from 'rxjs/operators'; import { Logger } from 'app/shared/logger.service';