From c397b59855a3869cfac8ddb321071fbe144e88a3 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Thu, 17 Jun 2021 23:58:15 +0300 Subject: [PATCH] test(docs-infra): ensure all redirect rules are tested (#42452) This commit adds a test assertion to verify that all redirect rules defined in `firebase.json` are tested, i.e. that each rule is applied to at least one testcase from `URLS_TO_REDIRECT.txt`. This will ensure that any redirect rules added in the future will be tested. PR Close #42452 --- aio/tests/deployment/shared/helpers.ts | 4 ++-- .../unit/testFirebaseRedirection.spec.ts | 12 ++++++++++- .../firebase-test-utils/FirebaseRedirect.ts | 3 ++- .../FirebaseRedirector.spec.ts | 21 +++++++++++++++++++ .../firebase-test-utils/FirebaseRedirector.ts | 8 +++++++ 5 files changed, 44 insertions(+), 4 deletions(-) diff --git a/aio/tests/deployment/shared/helpers.ts b/aio/tests/deployment/shared/helpers.ts index 03512dc463..fd0c0a6ba6 100644 --- a/aio/tests/deployment/shared/helpers.ts +++ b/aio/tests/deployment/shared/helpers.ts @@ -12,6 +12,7 @@ import { FirebaseRedirector, FirebaseRedirectConfig } from '../../../tools/fireb const AIO_DIR = resolvePath(__dirname, '../../..'); +export const PATH_TO_LEGACY_URLS = resolvePath(__dirname, 'URLS_TO_REDIRECT.txt'); export function getRedirector() { return new FirebaseRedirector(loadRedirects()); @@ -40,8 +41,7 @@ export function loadRedirects(): FirebaseRedirectConfig[] { } export function loadLegacyUrls() { - const pathToLegacyUrls = `${__dirname}/URLS_TO_REDIRECT.txt`; - const urls = readFileSync(pathToLegacyUrls, 'utf8') + const urls = readFileSync(PATH_TO_LEGACY_URLS, 'utf8') .split('\n') .filter(line => line.trim() !== '') .map(line => line.split(/\s*-->\s*/)); diff --git a/aio/tests/deployment/unit/testFirebaseRedirection.spec.ts b/aio/tests/deployment/unit/testFirebaseRedirection.spec.ts index e301c5c606..531ec16ed9 100644 --- a/aio/tests/deployment/unit/testFirebaseRedirection.spec.ts +++ b/aio/tests/deployment/unit/testFirebaseRedirection.spec.ts @@ -1,4 +1,6 @@ -import { getRedirector, loadLegacyUrls, loadLocalSitemapUrls, loadRedirects } from '../shared/helpers'; +import { + getRedirector, loadLegacyUrls, loadLocalSitemapUrls, loadRedirects, PATH_TO_LEGACY_URLS, +} from '../shared/helpers'; describe('firebase.json redirect config', () => { describe('with sitemap urls', () => { @@ -14,6 +16,14 @@ describe('firebase.json redirect config', () => { describe('with legacy urls', () => { const redirector = getRedirector(); + afterAll(() => { + expect(redirector.unusedRedirectConfigs) + .withContext( + 'Some redirect rules from \'firebase.json\' were not tested. ' + + `Ensure there is at least one testcase for each redirect rule in '${PATH_TO_LEGACY_URLS}'.`) + .toEqual([]); + }); + loadLegacyUrls().forEach(urlPair => { it(`should redirect legacy URL '${urlPair[0]}'`, () => { const redirected = redirector.redirect(urlPair[0]); diff --git a/aio/tools/firebase-test-utils/FirebaseRedirect.ts b/aio/tools/firebase-test-utils/FirebaseRedirect.ts index 3183dcd4b5..69c85f45cf 100644 --- a/aio/tools/firebase-test-utils/FirebaseRedirect.ts +++ b/aio/tools/firebase-test-utils/FirebaseRedirect.ts @@ -6,7 +6,8 @@ export class FirebaseRedirect { source: FirebaseRedirectSource; destination: string; - constructor({source, regex, destination}: FirebaseRedirectConfig) { + constructor(readonly rawConfig: FirebaseRedirectConfig) { + const {source, regex, destination} = rawConfig; this.source = (typeof source === 'string') ? FirebaseRedirectSource.fromGlobPattern(source) : FirebaseRedirectSource.fromRegexPattern(regex!); diff --git a/aio/tools/firebase-test-utils/FirebaseRedirector.spec.ts b/aio/tools/firebase-test-utils/FirebaseRedirector.spec.ts index 931f9b4933..03729967de 100644 --- a/aio/tools/firebase-test-utils/FirebaseRedirector.spec.ts +++ b/aio/tools/firebase-test-utils/FirebaseRedirector.spec.ts @@ -39,4 +39,25 @@ describe('FirebaseRedirector', () => { ]); expect(() => redirector.redirect('a')).toThrowError('infinite redirect loop'); }); + + it('should keep track of unused redirects', () => { + const redirects = [ + { source: '/a', destination: '/A' }, + { regex: '^/b$', destination: '/B' }, + { source: '/c', destination: '/C' }, + ]; + const redirector = new FirebaseRedirector(redirects); + + expect(redirector.unusedRedirectConfigs).toEqual([redirects[0], redirects[1], redirects[2]]); + + redirector.redirect('/a'); + expect(redirector.unusedRedirectConfigs).toEqual([redirects[1], redirects[2]]); + + redirector.redirect('/not-redirected'); + expect(redirector.unusedRedirectConfigs).toEqual([redirects[1], redirects[2]]); + + redirector.redirect('/b'); + redirector.redirect('/c'); + expect(redirector.unusedRedirectConfigs).toEqual([]); + }); }); diff --git a/aio/tools/firebase-test-utils/FirebaseRedirector.ts b/aio/tools/firebase-test-utils/FirebaseRedirector.ts index a525f789e9..d33c4c1706 100644 --- a/aio/tools/firebase-test-utils/FirebaseRedirector.ts +++ b/aio/tools/firebase-test-utils/FirebaseRedirector.ts @@ -6,8 +6,15 @@ export type FirebaseRedirectConfig = export class FirebaseRedirector { private redirects: FirebaseRedirect[]; + private unusedRedirects: Set; + + get unusedRedirectConfigs(): FirebaseRedirectConfig[] { + return [...this.unusedRedirects].map(({rawConfig}) => rawConfig); + } + constructor(redirects: FirebaseRedirectConfig[]) { this.redirects = redirects.map(redirect => new FirebaseRedirect(redirect)); + this.unusedRedirects = new Set(this.redirects); } redirect(url: string): string { @@ -28,6 +35,7 @@ export class FirebaseRedirector { for (const redirect of this.redirects) { const newUrl = redirect.replace(url); if (newUrl !== undefined) { + this.unusedRedirects.delete(redirect); return newUrl; } }