Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aio: add monitoring and fix a couple of issues #22483

Closed
wants to merge 10 commits into from
Closed
19 changes: 19 additions & 0 deletions .circleci/config.yml
Expand Up @@ -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
3 changes: 2 additions & 1 deletion aio/.gitignore
Expand Up @@ -30,6 +30,7 @@
/connect.lock
/coverage
/libpeerconnection.log
debug.log
npm-debug.log
testem.log
/typings
Expand All @@ -45,4 +46,4 @@ protractor-results*.txt
Thumbs.db

# copied dependencies
src/assets/js/lunr*
src/assets/js/lunr*
2 changes: 1 addition & 1 deletion aio/ngsw-manifest.json
Expand Up @@ -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"
}
}
Expand Down
12 changes: 6 additions & 6 deletions aio/package.json
Expand Up @@ -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 .",
Expand All @@ -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",
Expand All @@ -44,7 +45,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",
Expand All @@ -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",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT renaming ~update-webdriver to update-webdriver is the only real change in this file, right? Or am I missing something?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty much 😇

"~~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",
Expand Down
38 changes: 38 additions & 0 deletions 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!"
)
31 changes: 25 additions & 6 deletions 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';
Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -69,13 +80,15 @@ describe('SwUpdatesService', () => {
})));

it('should not pass a specific version to `NgServiceWorker.activateUpdate()`', fakeAsync(run(() => {
appRef.isStable.next(true);
sw.$$checkForUpdateSubj.next(true);
tick(checkInterval);

expect(sw.activateUpdate).toHaveBeenCalledWith(null);
})));

it('should schedule a new check after activating the update', fakeAsync(run(() => {
appRef.isStable.next(true);
sw.checkForUpdate.calls.reset();
sw.$$checkForUpdateSubj.next(true);

Expand Down Expand Up @@ -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();
Expand All @@ -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);
Expand Down Expand Up @@ -141,6 +156,10 @@ describe('SwUpdatesService', () => {
});

// Mocks
class MockApplicationRef {
isStable = new Subject<boolean>();
}

class MockLogger {
log = jasmine.createSpy('MockLogger.log');
}
Expand Down
29 changes: 15 additions & 14 deletions 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 { concat, Subject } from 'rxjs';
import { debounceTime, defaultIfEmpty, filter, first, map, startWith, takeUntil, tap } from 'rxjs/operators';

import { Logger } from 'app/shared/logger.service';


Expand Down Expand Up @@ -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<void>(undefined),
takeUntil(this.onDestroy),
)
constructor(appRef: ApplicationRef, private logger: Logger, private sw: NgServiceWorker) {
const appIsStable$ = appRef.isStable.pipe(first(v => v));
const checkForUpdates$ = this.checkForUpdateSubj.pipe(debounceTime(this.checkInterval), startWith<void>(undefined));

concat(appIsStable$, checkForUpdates$)
.pipe(takeUntil(this.onDestroy))
.subscribe(() => this.checkForUpdate());
}

Expand All @@ -51,11 +51,12 @@ 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(
take(1),
// 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}`)),
)
.subscribe(v => v ? this.activateUpdate() : this.scheduleCheckForUpdate());
Expand Down
2 changes: 1 addition & 1 deletion aio/src/main.ts
Expand Up @@ -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');
});
}
Expand Down
52 changes: 52 additions & 0 deletions aio/tests/deployment-config/e2e/protractor.conf.js
@@ -0,0 +1,52 @@
// 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]) => {
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}}));
});
}
};