From 372200bb1950c9dfafb86868aada7727b715672e Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 16 Apr 2020 16:49:53 +0200 Subject: [PATCH 01/10] github/workflows: split final cli lint and test task --- .github/workflows/cli.yml | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/.github/workflows/cli.yml b/.github/workflows/cli.yml index dad8437613..8ab722f29b 100644 --- a/.github/workflows/cli.yml +++ b/.github/workflows/cli.yml @@ -60,12 +60,17 @@ jobs: node ${{ github.workspace }}/scripts/cli-e2e-test.js env: BACKSTAGE_E2E_CLI_TEST: true - # This should lint and test both an app and a plugin - - name: yarn lint, test after creation + - name: lint newly created app and plugin working-directory: ${{ steps.generate_tempdir.outputs.tempdir }} run: | cd test-app yarn lint:all + env: + BACKSTAGE_E2E_CLI_TEST: true + - name: test newly created app and plugin + working-directory: ${{ steps.generate_tempdir.outputs.tempdir }} + run: | + cd test-app yarn test:all env: BACKSTAGE_E2E_CLI_TEST: true From bdafffb2371ad12873c016df845eb2fdd2ea4034 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 16 Apr 2020 16:52:47 +0200 Subject: [PATCH 02/10] packages/cli: no special handling of cli e2e tests wrt caching --- packages/cli/src/commands/build-cache/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/commands/build-cache/index.ts b/packages/cli/src/commands/build-cache/index.ts index a825a7f415..a8a1a27e9d 100644 --- a/packages/cli/src/commands/build-cache/index.ts +++ b/packages/cli/src/commands/build-cache/index.ts @@ -30,7 +30,7 @@ export async function withCache( buildFunc: () => Promise, ): Promise { const key = await Cache.readInputKey(options.inputs); - if (!key || process.env.BACKSTAGE_E2E_CLI_TEST) { + if (!key) { print('input directory is dirty, skipping cache'); await fs.remove(options.output); await buildFunc(); From fe0b3e5b10e1414eee2013a5bc310b80d19a7ae2 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 16 Apr 2020 17:12:22 +0200 Subject: [PATCH 03/10] packages/cli: added new ownRootDir to paths and use for create-app e2e override --- .../cli/src/commands/create-app/createApp.ts | 21 +++++-------- packages/cli/src/helpers/paths.ts | 31 +++++++++++++++++++ 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/packages/cli/src/commands/create-app/createApp.ts b/packages/cli/src/commands/create-app/createApp.ts index 219406704d..cb34dc0686 100644 --- a/packages/cli/src/commands/create-app/createApp.ts +++ b/packages/cli/src/commands/create-app/createApp.ts @@ -86,26 +86,22 @@ export async function moveApp( }); } -async function addPackageResolutions(rootDir: string, appDir: string) { - process.chdir(appDir); - - const packageFileContent = await fs.readFile('package.json', 'utf-8'); +async function addPackageResolutions(appDir: string) { + const pkgJsonPath = resolvePath(appDir, 'package.json'); + const packageFileContent = await fs.readFile(pkgJsonPath, 'utf-8'); const packageFileJson = JSON.parse(packageFileContent); - if (packageFileJson.resolutions) { - throw new Error('package.json already contains resolutions'); - } - packageFileJson.resolutions = {}; + packageFileJson.resolutions = packageFileJson.resolutions || {}; const packages = ['cli', 'core', 'test-utils', 'test-utils-core', 'theme']; for (const pkg of packages) { await Task.forItem('adding', `${pkg} link to package.json`, async () => { - const pkgPath = require('path').join(rootDir, 'packages', pkg); + const pkgPath = paths.resolveOwnRoot('packages', pkg); packageFileJson.resolutions[`@backstage/${pkg}`] = `file:${pkgPath}`; const newContents = `${JSON.stringify(packageFileJson, null, 2)}\n`; - await fs.writeFile('package.json', newContents, 'utf-8').catch(error => { + await fs.writeFile(pkgJsonPath, newContents, 'utf-8').catch(error => { throw new Error( `Failed to add resolutions to package.json: ${error.message}`, ); @@ -157,10 +153,7 @@ export default async () => { // e2e testing needs special treatment if (process.env.BACKSTAGE_E2E_CLI_TEST) { Task.section('Linking packages locally for e2e tests'); - const rootDir = process.env.CI - ? resolvePath(process.env.GITHUB_WORKSPACE!) - : resolvePath(__dirname, '..', '..', '..'); - await addPackageResolutions(rootDir, appDir); + await addPackageResolutions(appDir); } Task.section('Building the app'); diff --git a/packages/cli/src/helpers/paths.ts b/packages/cli/src/helpers/paths.ts index 173e77e1c5..e7b351ec16 100644 --- a/packages/cli/src/helpers/paths.ts +++ b/packages/cli/src/helpers/paths.ts @@ -25,6 +25,9 @@ export type Paths = { // Root dir of the cli itself, containing package.json ownDir: string; + // Monorepo root dir of the cli itself. Only accessible when running inside Backstage repo. + ownRoot: string; + // The location of the app that the cli is being executed in targetDir: string; @@ -34,6 +37,9 @@ export type Paths = { // Resolve a path relative to own repo resolveOwn: ResolveFunc; + // Resolve a path relative to own monorepo root. Only accessible when running inside Backstage repo. + resolveOwnRoot: ResolveFunc; + // Resolve a path relative to the app resolveTarget: ResolveFunc; @@ -91,10 +97,31 @@ export function findOwnDir() { return resolvePath(__dirname, path); } +// Finds the root of the monorepo that the cli exists in. Only accessible when running inside Backstage repo. +export function findOwnRootPath(ownDir: string) { + const isLocal = fs.pathExistsSync(resolvePath(ownDir, 'src')); + if (!isLocal) { + throw new Error( + 'Tried to access monorepo package root dir outside of Backstage repository', + ); + } + + return resolvePath(ownDir, '../..'); +} + export function findPaths(): Paths { const ownDir = findOwnDir(); const targetDir = fs.realpathSync(process.cwd()); + // Lazy load this as it will throw an error if we're not inside the Backstage repo. + let ownRoot = ''; + const getOwnRoot = () => { + if (!ownRoot) { + ownRoot = findOwnRootPath(ownDir); + } + return ownRoot; + }; + // We're not always running in a monorepo, so we lazy init this to only crash commands // that require a monorepo when we're not in one. let targetRoot = ''; @@ -107,11 +134,15 @@ export function findPaths(): Paths { return { ownDir, + get ownRoot() { + return getOwnRoot(); + }, targetDir, get targetRoot() { return getTargetRoot(); }, resolveOwn: (...paths) => resolvePath(ownDir, ...paths), + resolveOwnRoot: (...paths) => resolvePath(getOwnRoot(), ...paths), resolveTarget: (...paths) => resolvePath(targetDir, ...paths), resolveTargetRoot: (...paths) => resolvePath(getTargetRoot(), ...paths), }; From 54108354ff402b4cd25c545338bd847f0b388a7f Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 16 Apr 2020 17:52:08 +0200 Subject: [PATCH 04/10] github/workflows: avoid cd in final cli tests --- .github/workflows/cli.yml | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/.github/workflows/cli.yml b/.github/workflows/cli.yml index 8ab722f29b..d7cb262088 100644 --- a/.github/workflows/cli.yml +++ b/.github/workflows/cli.yml @@ -61,16 +61,12 @@ jobs: env: BACKSTAGE_E2E_CLI_TEST: true - name: lint newly created app and plugin - working-directory: ${{ steps.generate_tempdir.outputs.tempdir }} - run: | - cd test-app - yarn lint:all + run: yarn lint:all + working-directory: ${{ steps.generate_tempdir.outputs.tempdir }}/test-app env: BACKSTAGE_E2E_CLI_TEST: true - name: test newly created app and plugin - working-directory: ${{ steps.generate_tempdir.outputs.tempdir }} - run: | - cd test-app - yarn test:all + run: yarn test:all + working-directory: ${{ steps.generate_tempdir.outputs.tempdir }}/test-app env: BACKSTAGE_E2E_CLI_TEST: true From be0881ff35676b74fc3bfd4d9b26945521193b91 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 16 Apr 2020 18:46:30 +0200 Subject: [PATCH 05/10] scripts/cli-e2e-test: removed yarn init --- scripts/cli-e2e-test.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/scripts/cli-e2e-test.js b/scripts/cli-e2e-test.js index 4af26e2e41..92d73bfd6a 100644 --- a/scripts/cli-e2e-test.js +++ b/scripts/cli-e2e-test.js @@ -44,8 +44,6 @@ async function main() { process.chdir(tempDir); process.stdout.write(`Temp directory: ${process.cwd()}\n`); - await waitForExit(spawnPiped(['yarn', 'init --yes'])); - const createCmdPath = require('path').join( rootDir, 'packages', From 7a320302c10562bc60ffede310c26103145313a1 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 16 Apr 2020 18:54:50 +0200 Subject: [PATCH 06/10] scripts/cli-e2e-test: refactor path handling a bit --- scripts/cli-e2e-test.js | 13 +------------ scripts/createTestApp.js | 7 +++++-- scripts/generateTempDir.js | 7 ++++--- 3 files changed, 10 insertions(+), 17 deletions(-) diff --git a/scripts/cli-e2e-test.js b/scripts/cli-e2e-test.js index 92d73bfd6a..c73e812985 100644 --- a/scripts/cli-e2e-test.js +++ b/scripts/cli-e2e-test.js @@ -34,24 +34,13 @@ Browser.localhost('localhost', 3000); async function main() { process.env.BACKSTAGE_E2E_CLI_TEST = 'true'; - const rootDir = process.env.CI - ? resolvePath(process.env.GITHUB_WORKSPACE) - : resolvePath(__dirname, '..'); - const tempDir = process.env.CI ? process.cwd() : await generateTempDir(); process.stdout.write(`Initial directory: ${process.cwd()}\n`); process.chdir(tempDir); process.stdout.write(`Temp directory: ${process.cwd()}\n`); - const createCmdPath = require('path').join( - rootDir, - 'packages', - 'cli', - 'bin', - 'backstage-cli', - ); - await createTestApp(`${createCmdPath} create-app`); + await createTestApp(); const appDir = resolvePath(tempDir, 'test-app'); process.chdir(appDir); diff --git a/scripts/createTestApp.js b/scripts/createTestApp.js index 80f7bf3b18..88b9d772f1 100644 --- a/scripts/createTestApp.js +++ b/scripts/createTestApp.js @@ -14,11 +14,14 @@ * limitations under the License. */ +const { resolve: resolvePath } = require('path'); const { spawnPiped, waitFor, waitForExit, print } = require('./helpers'); -async function createTestApp(cmd) { +async function createTestApp() { + const cliPath = resolvePath(__dirname, '../packages/cli/bin/backstage-cli'); + print('Creating a Backstage App'); - const createApp = spawnPiped(['node', cmd]); + const createApp = spawnPiped(['node', cliPath, 'create-app']); try { let stdout = ''; diff --git a/scripts/generateTempDir.js b/scripts/generateTempDir.js index e455a29320..593c6eb74a 100644 --- a/scripts/generateTempDir.js +++ b/scripts/generateTempDir.js @@ -14,12 +14,13 @@ * limitations under the License. */ +const fs = require('fs-extra'); +const os = require('os'); +const { resolve: resolvePath } = require('path'); const { handleError } = require('./helpers'); async function generateTempDir() { - const tempDir = await require('fs-extra').mkdtemp( - require('path').join(require('os').tmpdir(), 'backstage-e2e-'), - ); + const tempDir = await fs.mkdtemp(resolvePath(os.tmpdir(), 'backstage-e2e-')); process.stdout.write(tempDir); return tempDir; } From e522989249388eddcf9b891a14b8d98050e9fab4 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 16 Apr 2020 19:06:11 +0200 Subject: [PATCH 07/10] github/workflows,scripts/cli-e2e-test: use runner temp dir instead of generated dir --- .github/workflows/cli.yml | 13 ++++--------- scripts/cli-e2e-test.js | 15 ++++++++++----- scripts/generateTempDir.js | 31 ------------------------------- 3 files changed, 14 insertions(+), 45 deletions(-) delete mode 100644 scripts/generateTempDir.js diff --git a/.github/workflows/cli.yml b/.github/workflows/cli.yml index d7cb262088..dea02ea206 100644 --- a/.github/workflows/cli.yml +++ b/.github/workflows/cli.yml @@ -41,19 +41,14 @@ jobs: - name: yarn install run: yarn install --frozen-lockfile - run: yarn build - # generate temp directory - - name: generate tempdir - id: generate_tempdir - run: echo "::set-output name=tempdir::$(node scripts/generateTempDir.js)" - # This creates a new app and plugin which pollutes the workspace, so it should be run last. - name: verify app and plugin creation on Windows - working-directory: ${{ steps.generate_tempdir.outputs.tempdir }} + working-directory: ${{ runner.temp }} if: runner.os == 'Windows' run: node ${{ github.workspace }}/scripts/cli-e2e-test.js env: BACKSTAGE_E2E_CLI_TEST: true - name: verify app and plugin creation on Linux - working-directory: ${{ steps.generate_tempdir.outputs.tempdir }} + working-directory: ${{ runner.temp }} if: runner.os == 'Linux' run: | sudo sysctl fs.inotify.max_user_watches=524288 @@ -62,11 +57,11 @@ jobs: BACKSTAGE_E2E_CLI_TEST: true - name: lint newly created app and plugin run: yarn lint:all - working-directory: ${{ steps.generate_tempdir.outputs.tempdir }}/test-app + working-directory: ${{ runner.temp }}/test-app env: BACKSTAGE_E2E_CLI_TEST: true - name: test newly created app and plugin run: yarn test:all - working-directory: ${{ steps.generate_tempdir.outputs.tempdir }}/test-app + working-directory: ${{ runner.temp }}/test-app env: BACKSTAGE_E2E_CLI_TEST: true diff --git a/scripts/cli-e2e-test.js b/scripts/cli-e2e-test.js index c73e812985..c8374af0eb 100644 --- a/scripts/cli-e2e-test.js +++ b/scripts/cli-e2e-test.js @@ -14,6 +14,8 @@ * limitations under the License. */ +const os = require('os'); +const fs = require('fs-extra'); const { resolve: resolvePath } = require('path'); const Browser = require('zombie'); @@ -27,22 +29,25 @@ const { const createTestApp = require('./createTestApp'); const createTestPlugin = require('./createTestPlugin'); -const generateTempDir = require('./generateTempDir.js'); Browser.localhost('localhost', 3000); +async function createTempDir() { + return fs.mkdtemp(resolvePath(os.tmpdir(), 'backstage-e2e-')); +} + async function main() { process.env.BACKSTAGE_E2E_CLI_TEST = 'true'; - const tempDir = process.env.CI ? process.cwd() : await generateTempDir(); + const workDir = process.env.CI ? process.cwd() : await createTempDir(); process.stdout.write(`Initial directory: ${process.cwd()}\n`); - process.chdir(tempDir); - process.stdout.write(`Temp directory: ${process.cwd()}\n`); + process.chdir(workDir); + process.stdout.write(`Working directory: ${process.cwd()}\n`); await createTestApp(); - const appDir = resolvePath(tempDir, 'test-app'); + const appDir = resolvePath(workDir, 'test-app'); process.chdir(appDir); process.stdout.write(`App directory: ${appDir}\n`); diff --git a/scripts/generateTempDir.js b/scripts/generateTempDir.js deleted file mode 100644 index 593c6eb74a..0000000000 --- a/scripts/generateTempDir.js +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Copyright 2020 Spotify AB - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -const fs = require('fs-extra'); -const os = require('os'); -const { resolve: resolvePath } = require('path'); -const { handleError } = require('./helpers'); - -async function generateTempDir() { - const tempDir = await fs.mkdtemp(resolvePath(os.tmpdir(), 'backstage-e2e-')); - process.stdout.write(tempDir); - return tempDir; -} - -module.exports = generateTempDir; - -process.on('unhandledRejection', handleError); -generateTempDir().catch(handleError); From d70337eb53dab9c8d2428100f88ef7915face173 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 16 Apr 2020 19:38:01 +0200 Subject: [PATCH 08/10] scripts: move cli-e2e-test to packages/cli/e2e-test --- .github/workflows/cli.yml | 4 +-- package.json | 3 +- packages/cli/e2e-test/.eslintrc.js | 29 +++++++++++++++++++ .../cli/e2e-test}/cli-e2e-test.js | 0 .../cli/e2e-test}/createTestApp.js | 2 +- .../cli/e2e-test}/createTestPlugin.js | 0 {scripts => packages/cli/e2e-test}/helpers.js | 0 packages/cli/package.json | 4 ++- 8 files changed, 36 insertions(+), 6 deletions(-) create mode 100644 packages/cli/e2e-test/.eslintrc.js rename {scripts => packages/cli/e2e-test}/cli-e2e-test.js (100%) rename {scripts => packages/cli/e2e-test}/createTestApp.js (94%) rename {scripts => packages/cli/e2e-test}/createTestPlugin.js (100%) rename {scripts => packages/cli/e2e-test}/helpers.js (100%) diff --git a/.github/workflows/cli.yml b/.github/workflows/cli.yml index dea02ea206..612375ca1c 100644 --- a/.github/workflows/cli.yml +++ b/.github/workflows/cli.yml @@ -44,7 +44,7 @@ jobs: - name: verify app and plugin creation on Windows working-directory: ${{ runner.temp }} if: runner.os == 'Windows' - run: node ${{ github.workspace }}/scripts/cli-e2e-test.js + run: node ${{ github.workspace }}/packages/cli/e2e-test/cli-e2e-test.js env: BACKSTAGE_E2E_CLI_TEST: true - name: verify app and plugin creation on Linux @@ -52,7 +52,7 @@ jobs: if: runner.os == 'Linux' run: | sudo sysctl fs.inotify.max_user_watches=524288 - node ${{ github.workspace }}/scripts/cli-e2e-test.js + node ${{ github.workspace }}/packages/cli/e2e-test/cli-e2e-test.js env: BACKSTAGE_E2E_CLI_TEST: true - name: lint newly created app and plugin diff --git a/package.json b/package.json index 10a5daa4c1..f04c982621 100644 --- a/package.json +++ b/package.json @@ -33,8 +33,7 @@ "lerna": "^3.20.2", "lint-staged": "^10.1.0", "prettier": "^1.19.1", - "typescript": "^3.7.5", - "zombie": "^6.1.4" + "typescript": "^3.7.5" }, "dependencies": { "@types/classnames": "^2.2.9", diff --git a/packages/cli/e2e-test/.eslintrc.js b/packages/cli/e2e-test/.eslintrc.js new file mode 100644 index 0000000000..274c7426b8 --- /dev/null +++ b/packages/cli/e2e-test/.eslintrc.js @@ -0,0 +1,29 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +module.exports = { + rules: { + 'import/no-extraneous-dependencies': [ + 'error', + { + devDependencies: true, + optionalDependencies: true, + peerDependencies: true, + bundledDependencies: true, + }, + ], + }, +}; diff --git a/scripts/cli-e2e-test.js b/packages/cli/e2e-test/cli-e2e-test.js similarity index 100% rename from scripts/cli-e2e-test.js rename to packages/cli/e2e-test/cli-e2e-test.js diff --git a/scripts/createTestApp.js b/packages/cli/e2e-test/createTestApp.js similarity index 94% rename from scripts/createTestApp.js rename to packages/cli/e2e-test/createTestApp.js index 88b9d772f1..5437804975 100644 --- a/scripts/createTestApp.js +++ b/packages/cli/e2e-test/createTestApp.js @@ -18,7 +18,7 @@ const { resolve: resolvePath } = require('path'); const { spawnPiped, waitFor, waitForExit, print } = require('./helpers'); async function createTestApp() { - const cliPath = resolvePath(__dirname, '../packages/cli/bin/backstage-cli'); + const cliPath = resolvePath(__dirname, '../bin/backstage-cli'); print('Creating a Backstage App'); const createApp = spawnPiped(['node', cliPath, 'create-app']); diff --git a/scripts/createTestPlugin.js b/packages/cli/e2e-test/createTestPlugin.js similarity index 100% rename from scripts/createTestPlugin.js rename to packages/cli/e2e-test/createTestPlugin.js diff --git a/scripts/helpers.js b/packages/cli/e2e-test/helpers.js similarity index 100% rename from scripts/helpers.js rename to packages/cli/e2e-test/helpers.js diff --git a/packages/cli/package.json b/packages/cli/package.json index 2a2adba6a8..faf8710d14 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -22,6 +22,7 @@ "build": "backstage-cli build-cache -- tsc", "lint": "backstage-cli lint", "test": "backstage-cli test", + "test:e2e": "node e2e-test/cli-e2e-test.js", "clean": "backstage-cli clean", "start": "nodemon ." }, @@ -41,7 +42,8 @@ "del": "^5.1.0", "nodemon": "^2.0.2", "ts-node": "^8.6.2", - "tsconfig-paths": "^3.9.0" + "tsconfig-paths": "^3.9.0", + "zombie": "^6.1.4" }, "bin": { "backstage-cli": "bin/backstage-cli" From 40b4ea835bbf9c422d754f394dfd46eb49aeb464 Mon Sep 17 00:00:00 2001 From: Jose Balanza Martinez Date: Thu, 16 Apr 2020 13:29:26 -0500 Subject: [PATCH 09/10] Update logic for create app --- .../commands/create-plugin/createPlugin.ts | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/packages/cli/src/commands/create-plugin/createPlugin.ts b/packages/cli/src/commands/create-plugin/createPlugin.ts index 51f749a6ac..f1c0230e09 100644 --- a/packages/cli/src/commands/create-plugin/createPlugin.ts +++ b/packages/cli/src/commands/create-plugin/createPlugin.ts @@ -68,16 +68,12 @@ const sortObjectByKeys = (obj: { [name in string]: string }) => { const capitalize = (str: string): string => str.charAt(0).toUpperCase() + str.slice(1); -const addExportStatement = async ( - file: string, - importStatement: string, - exportStatement: string, -) => { +const addExportStatement = async (file: string, exportStatement: string) => { const newContents = fs .readFileSync(file, 'utf8') .split('\n') .filter(Boolean) // get rid of empty lines - .concat([importStatement, exportStatement]) + .concat([exportStatement]) .concat(['']) // newline at end of file .join('\n'); @@ -122,19 +118,16 @@ export async function addPluginToApp(rootDir: string, pluginName: string) { .split('-') .map(name => capitalize(name)) .join(''); - const pluginImport = `import { plugin as ${pluginNameCapitalized} } from '${pluginPackage}';`; - const pluginExport = `export { ${pluginNameCapitalized} };`; + const pluginExport = `export { plugin as ${pluginNameCapitalized} } from '${pluginPackage}';`; const pluginsFilePath = 'packages/app/src/plugins.ts'; const pluginsFile = resolvePath(rootDir, pluginsFilePath); await Task.forItem('processing', pluginsFilePath, async () => { - await addExportStatement(pluginsFile, pluginImport, pluginExport).catch( - error => { - throw new Error( - `Failed to import plugin in app: ${pluginsFile}: ${error.message}`, - ); - }, - ); + await addExportStatement(pluginsFile, pluginExport).catch(error => { + throw new Error( + `Failed to import plugin in app: ${pluginsFile}: ${error.message}`, + ); + }); }); } From f774c115dc02e9739f1f3e5d9330eb80c072fb07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20=C3=85lund?= Date: Fri, 17 Apr 2020 08:25:20 +0200 Subject: [PATCH 10/10] Extract TrendLine from Lighthouse plugin (#532) * Extract TrendLine from Lighthouse plugin * Create TrendLine.stories.tsx * Change lifecycle names * Made story example smaller * Review comment * Update TrendLine.tsx --- packages/core/package.json | 2 + .../Lifecycle/LifecycleAlpha.stories.tsx | 2 +- .../Lifecycle/LifecycleBeta.stories.tsx | 2 +- .../TrendLine/TrendLine.stories.tsx | 43 +++++++++++++++++++ .../components/TrendLine/TrendLine.test.tsx | 23 +++------- .../src/components/TrendLine/TrendLine.tsx | 5 ++- .../core/src/components/TrendLine/index.ts | 17 ++++++++ packages/core/src/index.ts | 1 + plugins/lighthouse/package.json | 4 +- .../components/AuditList/AuditListTable.tsx | 4 +- 10 files changed, 78 insertions(+), 25 deletions(-) create mode 100644 packages/core/src/components/TrendLine/TrendLine.stories.tsx rename plugins/lighthouse/src/components/CategoryTrendline/index.test.tsx => packages/core/src/components/TrendLine/TrendLine.test.tsx (77%) rename plugins/lighthouse/src/components/CategoryTrendline/index.tsx => packages/core/src/components/TrendLine/TrendLine.tsx (92%) create mode 100644 packages/core/src/components/TrendLine/index.ts diff --git a/packages/core/package.json b/packages/core/package.json index 609f00e2d0..651ac1006c 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -39,6 +39,7 @@ "react-dom": "^16.12.0", "react-helmet": "5.2.1", "react-router-dom": "^5.1.2", + "react-sparklines": "^1.7.0", "recompose": "0.30.0" }, "devDependencies": { @@ -49,6 +50,7 @@ "@testing-library/jest-dom": "^4.2.4", "@testing-library/react": "^9.3.2", "@testing-library/user-event": "^7.1.2", + "@types/react-sparklines": "^1.7.0", "react-router": "^5.1.2" }, "peerDependencies": { diff --git a/packages/core/src/components/Lifecycle/LifecycleAlpha.stories.tsx b/packages/core/src/components/Lifecycle/LifecycleAlpha.stories.tsx index 1b044fcd5f..072fb90a55 100644 --- a/packages/core/src/components/Lifecycle/LifecycleAlpha.stories.tsx +++ b/packages/core/src/components/Lifecycle/LifecycleAlpha.stories.tsx @@ -17,7 +17,7 @@ import React from 'react'; import { AlphaLabel } from './Lifecycle'; export default { - title: 'Alpha Lifecycle', + title: 'Lifecycle - Alpha', component: AlphaLabel, }; diff --git a/packages/core/src/components/Lifecycle/LifecycleBeta.stories.tsx b/packages/core/src/components/Lifecycle/LifecycleBeta.stories.tsx index b74171b5ea..f8cce2a562 100644 --- a/packages/core/src/components/Lifecycle/LifecycleBeta.stories.tsx +++ b/packages/core/src/components/Lifecycle/LifecycleBeta.stories.tsx @@ -17,7 +17,7 @@ import React from 'react'; import { BetaLabel } from './Lifecycle'; export default { - title: 'Beta Lifecycle', + title: 'Lifecycle - Beta', component: BetaLabel, }; diff --git a/packages/core/src/components/TrendLine/TrendLine.stories.tsx b/packages/core/src/components/TrendLine/TrendLine.stories.tsx new file mode 100644 index 0000000000..855b14207c --- /dev/null +++ b/packages/core/src/components/TrendLine/TrendLine.stories.tsx @@ -0,0 +1,43 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React from 'react'; +import TrendLine from '.'; + +export default { + title: 'TrendLine', + component: TrendLine, +}; + +const width = 140; + +export const Default = () => ( +
+ +
+); + +export const TrendingUp = () => ( +
+ +
+); + +export const TrendingDown = () => ( +
+ +
+); diff --git a/plugins/lighthouse/src/components/CategoryTrendline/index.test.tsx b/packages/core/src/components/TrendLine/TrendLine.test.tsx similarity index 77% rename from plugins/lighthouse/src/components/CategoryTrendline/index.test.tsx rename to packages/core/src/components/TrendLine/TrendLine.test.tsx index c45db58b9e..985e5d2b31 100644 --- a/plugins/lighthouse/src/components/CategoryTrendline/index.test.tsx +++ b/packages/core/src/components/TrendLine/TrendLine.test.tsx @@ -15,18 +15,17 @@ */ /* eslint-disable jest/no-disabled-tests */ - import React from 'react'; import { render } from '@testing-library/react'; import { wrapInThemedTestApp } from '@backstage/test-utils'; -import CategoryTrendline from '.'; +import TrendLine from '.'; -describe('CategoryTrendline', () => { +describe('TrendLine', () => { describe('when no data is present', () => { it('renders null without throwing', () => { const rendered = render( - wrapInThemedTestApp(), + wrapInThemedTestApp(), ); expect(rendered.queryByTitle('sparkline')).not.toBeInTheDocument(); }); @@ -35,9 +34,7 @@ describe('CategoryTrendline', () => { describe('when one datapoint is present', () => { it('renders as a straight line', () => { const rendered = render( - wrapInThemedTestApp( - , - ), + wrapInThemedTestApp(), ); expect(rendered.getByTitle('sparkline')).toBeInTheDocument(); }); @@ -46,9 +43,7 @@ describe('CategoryTrendline', () => { describe.skip('when the data finishes above the success threshold', () => { it('renders with the correct color', () => { const rendered = render( - wrapInThemedTestApp( - , - ), + wrapInThemedTestApp(), ); expect(rendered.getByTitle('sparkline')).toBeInTheDocument(); }); @@ -57,9 +52,7 @@ describe('CategoryTrendline', () => { describe.skip('when the data finishes within the the warning threshold', () => { it('renders with the correct color', () => { const rendered = render( - wrapInThemedTestApp( - , - ), + wrapInThemedTestApp(), ); expect(rendered.getByTitle('sparkline')).toBeInTheDocument(); }); @@ -68,9 +61,7 @@ describe('CategoryTrendline', () => { describe.skip('when the data finishes within the the error threshold', () => { it('renders with the correct color', () => { const rendered = render( - wrapInThemedTestApp( - , - ), + wrapInThemedTestApp(), ); expect(rendered.getByTitle('sparkline')).toBeInTheDocument(); }); diff --git a/plugins/lighthouse/src/components/CategoryTrendline/index.tsx b/packages/core/src/components/TrendLine/TrendLine.tsx similarity index 92% rename from plugins/lighthouse/src/components/CategoryTrendline/index.tsx rename to packages/core/src/components/TrendLine/TrendLine.tsx index e3dbd186e0..3ab9b67e53 100644 --- a/plugins/lighthouse/src/components/CategoryTrendline/index.tsx +++ b/packages/core/src/components/TrendLine/TrendLine.tsx @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import React, { FC } from 'react'; import { Sparklines, SparklinesLine, SparklinesProps } from 'react-sparklines'; import { useTheme } from '@material-ui/core'; @@ -26,7 +27,7 @@ function color(data: number[], theme: BackstageTheme): string | undefined { return theme.palette.status.error; } -const CategoryTrendline: FC = props => { +const Trendline: FC = props => { const theme = useTheme(); if (!props.data) return null; @@ -38,4 +39,4 @@ const CategoryTrendline: FC = props => { ); }; -export default CategoryTrendline; +export default Trendline; diff --git a/packages/core/src/components/TrendLine/index.ts b/packages/core/src/components/TrendLine/index.ts new file mode 100644 index 0000000000..168c6e6d3f --- /dev/null +++ b/packages/core/src/components/TrendLine/index.ts @@ -0,0 +1,17 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export { default } from './TrendLine'; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 9153b9fa3b..a13146991c 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -31,6 +31,7 @@ export { default as Progress } from './components/Progress'; export { AlphaLabel, BetaLabel } from './components/Lifecycle'; export { default as SupportButton } from './components/SupportButton'; export { default as SortableTable } from './components/SortableTable'; +export { default as TrendLine } from './components/TrendLine'; export { FeatureCalloutCircular } from './components/FeatureDiscovery/FeatureCalloutCircular'; export * from './components/Status'; export { default as WarningPanel } from './components/WarningPanel'; diff --git a/plugins/lighthouse/package.json b/plugins/lighthouse/package.json index 442211a38c..d082b0be0f 100644 --- a/plugins/lighthouse/package.json +++ b/plugins/lighthouse/package.json @@ -13,8 +13,7 @@ "clean": "backstage-cli clean" }, "dependencies": { - "react-markdown": "^4.3.1", - "react-sparklines": "^1.7.0" + "react-markdown": "^4.3.1" }, "devDependencies": { "@backstage/cli": "^0.1.1-alpha.4", @@ -29,7 +28,6 @@ "@testing-library/user-event": "^7.1.2", "@types/jest": "^24.0.0", "@types/node": "^12.0.0", - "@types/react-sparklines": "^1.7.0", "@types/testing-library__jest-dom": "5.0.2", "jest-fetch-mock": "^3.0.3", "react": "^16.13.1", diff --git a/plugins/lighthouse/src/components/AuditList/AuditListTable.tsx b/plugins/lighthouse/src/components/AuditList/AuditListTable.tsx index 52f3eeb5e2..ba0e79f066 100644 --- a/plugins/lighthouse/src/components/AuditList/AuditListTable.tsx +++ b/plugins/lighthouse/src/components/AuditList/AuditListTable.tsx @@ -24,6 +24,7 @@ import { TableRow, } from '@material-ui/core'; import { makeStyles } from '@material-ui/core/styles'; +import { TrendLine } from '@backstage/core'; import { Audit, @@ -32,7 +33,6 @@ import { Website, } from '../../api'; import { formatTime } from '../../utils'; -import CategoryTrendline from '../CategoryTrendline'; import AuditStatusIcon from '../AuditStatusIcon'; export const CATEGORIES: LighthouseCategoryId[] = [ @@ -132,7 +132,7 @@ export const AuditListTable: FC<{ items: Website[] }> = ({ items }) => { key={`${website.url}|${category}`} className={classes.sparklinesCell} > -