From ea0e5db06bf71f7c2afff0471a0499ec6947d9f2 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 26 Jun 2020 17:25:59 +0200 Subject: [PATCH 01/10] cli: add build-workspace command --- packages/cli/src/commands/buildWorkspace.ts | 29 +++++++++++++++++++++ packages/cli/src/index.ts | 5 ++++ packages/cli/src/lib/packager/index.ts | 2 +- 3 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 packages/cli/src/commands/buildWorkspace.ts diff --git a/packages/cli/src/commands/buildWorkspace.ts b/packages/cli/src/commands/buildWorkspace.ts new file mode 100644 index 0000000000..624f104b3f --- /dev/null +++ b/packages/cli/src/commands/buildWorkspace.ts @@ -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. + */ + +import fs from 'fs-extra'; +import { Command } from 'commander'; +import { createDistWorkspace } from '../lib/packager'; + +export default async (dir: string, _cmd: Command, packages: string[]) => { + if (!(await fs.pathExists(dir))) { + throw new Error(`Target workspace directory doesn't exist, '${dir}'`); + } + + await createDistWorkspace(packages, { + targetDir: dir, + }); +}; diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index bed32bcf8b..f29a1c1005 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -140,6 +140,11 @@ const main = (argv: string[]) => { .description('Delete cache directories') .action(lazyAction(() => import('./commands/clean/clean'), 'default')); + program + .command('build-workspace ...') + .description('Builds a temporary dist workspace from the provided packages') + .action(lazyAction(() => import('./commands/buildWorkspace'), 'default')); + program.on('command:*', () => { console.log(); console.log( diff --git a/packages/cli/src/lib/packager/index.ts b/packages/cli/src/lib/packager/index.ts index 5e309ab953..d3ba1f6b12 100644 --- a/packages/cli/src/lib/packager/index.ts +++ b/packages/cli/src/lib/packager/index.ts @@ -59,7 +59,7 @@ type Options = { */ export async function createDistWorkspace( packageNames: string[], - options: Options, + options: Options = {}, ) { const targetDir = options.targetDir ?? From c01de0d489e7f5b9e075d709741e791a39a93dfc Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 29 Jun 2020 13:47:13 +0200 Subject: [PATCH 02/10] cli: add --skip-install option to create-app --- .../cli/src/commands/create-app/createApp.ts | 17 ++++++++--------- packages/cli/src/index.ts | 4 ++++ 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/packages/cli/src/commands/create-app/createApp.ts b/packages/cli/src/commands/create-app/createApp.ts index f3ac37667f..eed078342f 100644 --- a/packages/cli/src/commands/create-app/createApp.ts +++ b/packages/cli/src/commands/create-app/createApp.ts @@ -17,6 +17,7 @@ import fs from 'fs-extra'; import { promisify } from 'util'; import chalk from 'chalk'; +import { Command } from 'commander'; import inquirer, { Answers, Question } from 'inquirer'; import { exec as execCb } from 'child_process'; import { resolve as resolvePath } from 'path'; @@ -40,7 +41,7 @@ async function checkExists(rootDir: string, name: string) { }); } -export async function createTemporaryAppFolder(tempDir: string) { +async function createTemporaryAppFolder(tempDir: string) { await Task.forItem('creating', 'temporary directory', async () => { try { await fs.mkdir(tempDir); @@ -76,11 +77,7 @@ async function buildApp(appDir: string) { await runCmd('yarn build'); } -export async function moveApp( - tempDir: string, - destination: string, - id: string, -) { +async function moveApp(tempDir: string, destination: string, id: string) { await Task.forItem('moving', id, async () => { await fs.move(tempDir, destination).catch(error => { throw new Error( @@ -90,7 +87,7 @@ export async function moveApp( }); } -export default async () => { +export default async (cmd: Command): Promise => { const questions: Question[] = [ { type: 'input', @@ -130,8 +127,10 @@ export default async () => { Task.section('Moving to final location'); await moveApp(tempDir, appDir, answers.name); - Task.section('Building the app'); - await buildApp(appDir); + if (!cmd.skipInstall) { + Task.section('Building the app'); + await buildApp(appDir); + } Task.log(); Task.log( diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index f29a1c1005..9a102ab7d3 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -25,6 +25,10 @@ const main = (argv: string[]) => { program .command('create-app') .description('Creates a new app in a new directory') + .option( + '--skip-install', + 'Skip the install and builds steps after creating the app', + ) .action( lazyAction(() => import('./commands/create-app/createApp'), 'default'), ); From 3c447e36dcb622f993527f4fea876638ee799bee Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 2 Jul 2020 01:03:57 +0200 Subject: [PATCH 03/10] cli: update e2e-test to use build-workspace and inline more scripts --- packages/cli/e2e-test/cli-e2e-test.js | 225 +++++++++++++++++++--- packages/cli/e2e-test/createTestApp.js | 44 ----- packages/cli/e2e-test/createTestPlugin.js | 44 ----- packages/cli/e2e-test/helpers.js | 6 +- 4 files changed, 199 insertions(+), 120 deletions(-) delete mode 100644 packages/cli/e2e-test/createTestApp.js delete mode 100644 packages/cli/e2e-test/createTestPlugin.js diff --git a/packages/cli/e2e-test/cli-e2e-test.js b/packages/cli/e2e-test/cli-e2e-test.js index c8374af0eb..ff0e9193b0 100644 --- a/packages/cli/e2e-test/cli-e2e-test.js +++ b/packages/cli/e2e-test/cli-e2e-test.js @@ -16,45 +16,210 @@ const os = require('os'); const fs = require('fs-extra'); -const { resolve: resolvePath } = require('path'); +const { resolve: resolvePath, join: joinPath } = require('path'); const Browser = require('zombie'); - +const { execFile: execFileCb } = require('child_process'); +const { promisify } = require('util'); const { spawnPiped, handleError, waitForPageWithText, + waitFor, waitForExit, print, } = require('./helpers'); - -const createTestApp = require('./createTestApp'); -const createTestPlugin = require('./createTestPlugin'); - -Browser.localhost('localhost', 3000); - -async function createTempDir() { - return fs.mkdtemp(resolvePath(os.tmpdir(), 'backstage-e2e-')); -} +const execFile = promisify(execFileCb); async function main() { - process.env.BACKSTAGE_E2E_CLI_TEST = 'true'; + const rootDir = await fs.mkdtemp(resolvePath(os.tmpdir(), 'backstage-e2e-')); + print(`CLI E2E test root: ${rootDir}\n`); - const workDir = process.env.CI ? process.cwd() : await createTempDir(); + print('Building dist workspace'); + const workspaceDir = await buildDistWorkspace('workspace', rootDir); - process.stdout.write(`Initial directory: ${process.cwd()}\n`); - process.chdir(workDir); - process.stdout.write(`Working directory: ${process.cwd()}\n`); + print('Creating a Backstage App'); + const appDir = await createApp('test-app', workspaceDir, rootDir); - await createTestApp(); - - const appDir = resolvePath(workDir, 'test-app'); - process.chdir(appDir); - process.stdout.write(`App directory: ${appDir}\n`); - - await createTestPlugin(); + print('Creating a Backstage Plugin'); + const pluginName = await createPlugin('test-plugin', appDir); print('Starting the app'); - const startApp = spawnPiped(['yarn', 'start']); + await testAppServe(pluginName, appDir); + + print('All tests successful, removing test dir'); + await fs.remove(rootDir); +} + +/** + * Builds a dist workspace that contains the cli and core packages + */ +async function buildDistWorkspace(workspaceName, rootDir) { + const workspaceDir = resolvePath(rootDir, workspaceName); + await fs.ensureDir(workspaceDir); + + print(`Preparing workspace`); + await execFile('yarn', [ + 'backstage-cli', + 'build-workspace', + workspaceDir, + '@backstage/cli', + '@backstage/core', + '@backstage/dev-utils', + '@backstage/test-utils', + ]); + + print('Pinning yarn version in workspace'); + await pinYarnVersion(workspaceDir); + + print('Installing workspace dependencies'); + await execFile('yarn', ['install', '--production', '--frozen-lockfile'], { + cwd: workspaceDir, + }); + + return workspaceDir; +} + +/** + * Pin the yarn version in a directory to the one we're using in the Backstage repo + */ +async function pinYarnVersion(dir) { + const repoRoot = resolvePath(__dirname, '../../..'); + + const yarnRc = await fs.readFile(resolvePath(repoRoot, '.yarnrc'), 'utf8'); + const yarnRcLines = yarnRc.split('\n'); + const yarnPathLine = yarnRcLines.find(line => line.startsWith('yarn-path')); + const [, localYarnPath] = yarnPathLine.match(/"(.*)"/); + const yarnPath = resolvePath(repoRoot, localYarnPath); + + await fs.writeFile(resolvePath(dir, '.yarnrc'), `yarn-path "${yarnPath}"\n`); +} + +/** + * Creates a new app inside rootDir called test-app, using packages from the workspaceDir + */ +async function createApp(appName, workspaceDir, rootDir) { + const child = spawnPiped( + [ + resolvePath(workspaceDir, 'packages/cli/bin/backstage-cli'), + 'create-app', + '--skip-install', + ], + { + cwd: rootDir, + }, + ); + + try { + let stdout = ''; + child.stdout.on('data', data => { + stdout = stdout + data.toString('utf8'); + }); + + await waitFor(() => stdout.includes('Enter a name for the app')); + child.stdin.write(`${appName}\n`); + + print('Waiting for app create script to be done'); + await waitForExit(child); + + const appDir = resolvePath(rootDir, appName); + + print('Rewriting module resolutions of app to use workspace packages'); + await overrideModuleResolutions(appDir, workspaceDir); + + print('Pinning yarn version and registry in app'); + await pinYarnVersion(appDir); + await fs.writeFile( + resolvePath(appDir, '.npmrc'), + 'registry=https://registry.npmjs.org/\n', + ); + + print('Test app created'); + + for (const cmd of ['install', 'tsc', 'build', 'lint:all', 'test:all']) { + print(`Running 'yarn ${cmd}' in newly created app`); + await execFile('yarn', [cmd], { cwd: appDir }); + } + + print(`Running 'yarn test:e2e:ci' in newly created app`); + await execFile('yarn', ['test:e2e:ci'], { + cwd: resolvePath(appDir, 'packages', 'app'), + env: { + ...process.env, + APP_CONFIG_app_baseUrl: '"http://localhost:3001"', + }, + }); + + return appDir; + } finally { + child.kill(); + } +} + +/** + * This points dependency resolutions into the workspace for each package that is present there + */ +async function overrideModuleResolutions(appDir, workspaceDir) { + const pkgJsonPath = resolvePath(appDir, 'package.json'); + const pkgJson = await fs.readJson(pkgJsonPath); + + pkgJson.resolutions = pkgJson.resolutions || {}; + pkgJson.dependencies = pkgJson.dependencies || {}; + + const packageNames = await fs.readdir(resolvePath(workspaceDir, 'packages')); + for (const name of packageNames) { + const pkgPath = joinPath('..', 'workspace', 'packages', name); + + pkgJson.dependencies[`@backstage/${name}`] = `file:${pkgPath}`; + pkgJson.resolutions[`@backstage/${name}`] = `file:${pkgPath}`; + delete pkgJson.devDependencies[`@backstage/${name}`]; + } + fs.writeJson(pkgJsonPath, pkgJson, { spaces: 2 }); +} + +/** + * Uses create-plugin command to create a new plugin in the app + */ +async function createPlugin(pluginName, appDir) { + const child = spawnPiped(['yarn', 'create-plugin'], { + cwd: appDir, + }); + + try { + let stdout = ''; + child.stdout.on('data', data => { + stdout = stdout + data.toString('utf8'); + }); + + await waitFor(() => stdout.includes('Enter an ID for the plugin')); + child.stdin.write(`${pluginName}\n`); + + // await waitFor(() => stdout.includes('Enter the owner(s) of the plugin')); + // child.stdin.write('@someuser\n'); + + print('Waiting for plugin create script to be done'); + await waitForExit(child); + + const pluginDir = resolvePath(appDir, 'plugins', pluginName); + for (const cmd of [['lint'], ['test', '--no-watch']]) { + print(`Running 'yarn ${cmd.join(' ')}' in newly created plugin`); + await execFile('yarn', cmd, { cwd: pluginDir }); + } + + return pluginName; + } finally { + child.kill(); + } +} + +/** + * Start serving the newly created app and make sure that the create plugin is rendering correctly + */ +async function testAppServe(pluginName, appDir) { + const startApp = spawnPiped(['yarn', 'start'], { + cwd: appDir, + detached: true, + }); + Browser.localhost('localhost', 3000); try { const browser = new Browser(); @@ -62,19 +227,19 @@ async function main() { await waitForPageWithText(browser, '/', 'Welcome to Backstage'); await waitForPageWithText( browser, - '/test-plugin', - 'Welcome to test-plugin!', + `/${pluginName}`, + `Welcome to ${pluginName}!`, ); print('Both App and Plugin loaded correctly'); + } catch (error) { + throw new Error(`App serve test failed, ${error}`); } finally { - startApp.kill(); + // Kill entire process group, otherwise we'll end up with hanging serve processes + process.kill(-startApp.pid, 'SIGTERM'); } await waitForExit(startApp); - - print('All tests done'); - process.exit(0); } process.on('unhandledRejection', handleError); diff --git a/packages/cli/e2e-test/createTestApp.js b/packages/cli/e2e-test/createTestApp.js deleted file mode 100644 index 5437804975..0000000000 --- a/packages/cli/e2e-test/createTestApp.js +++ /dev/null @@ -1,44 +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 { resolve: resolvePath } = require('path'); -const { spawnPiped, waitFor, waitForExit, print } = require('./helpers'); - -async function createTestApp() { - const cliPath = resolvePath(__dirname, '../bin/backstage-cli'); - - print('Creating a Backstage App'); - const createApp = spawnPiped(['node', cliPath, 'create-app']); - - try { - let stdout = ''; - createApp.stdout.on('data', data => { - stdout = stdout + data.toString('utf8'); - }); - - await waitFor(() => stdout.includes('Enter a name for the app')); - createApp.stdin.write('test-app\n'); - - print('Waiting for app create script to be done'); - await waitForExit(createApp); - - print('Test app created'); - } finally { - createApp.kill(); - } -} - -module.exports = createTestApp; diff --git a/packages/cli/e2e-test/createTestPlugin.js b/packages/cli/e2e-test/createTestPlugin.js deleted file mode 100644 index 59e8d6dd77..0000000000 --- a/packages/cli/e2e-test/createTestPlugin.js +++ /dev/null @@ -1,44 +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 { spawnPiped, waitFor, waitForExit, print } = require('./helpers'); - -async function createTestPlugin() { - print('Creating a Backstage Plugin'); - const createPlugin = spawnPiped(['yarn', 'create-plugin']); - - try { - let stdout = ''; - createPlugin.stdout.on('data', data => { - stdout = stdout + data.toString('utf8'); - }); - - await waitFor(() => stdout.includes('Enter an ID for the plugin')); - createPlugin.stdin.write('test-plugin\n'); - - // await waitFor(() => stdout.includes('Enter the owner(s) of the plugin')); - // createPlugin.stdin.write('@someuser\n'); - - print('Waiting for plugin create script to be done'); - await waitForExit(createPlugin); - - print('Test plugin created'); - } finally { - createPlugin.kill(); - } -} - -module.exports = createTestPlugin; diff --git a/packages/cli/e2e-test/helpers.js b/packages/cli/e2e-test/helpers.js index d7f3f28a7a..1292ba19bf 100644 --- a/packages/cli/e2e-test/helpers.js +++ b/packages/cli/e2e-test/helpers.js @@ -43,13 +43,15 @@ function spawnPiped(cmd, options) { process.exit(code); } }); + + const logPrefix = cmd.map(s => s.replace(/.+\//, '')).join(' '); child.stdout.on( 'data', - pipeWithPrefix(process.stdout, `[${cmd.join(' ')}].out: `), + pipeWithPrefix(process.stdout, `[${logPrefix}].out: `), ); child.stderr.on( 'data', - pipeWithPrefix(process.stderr, `[${cmd.join(' ')}].err: `), + pipeWithPrefix(process.stderr, `[${logPrefix}].err: `), ); return child; From 3fb76b575b6a927e77ffe871ae56c9c90318cf31 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 2 Jul 2020 01:19:40 +0200 Subject: [PATCH 04/10] cli: remove special handling of yarn install --- .../cli/src/commands/create-app/createApp.ts | 4 +- .../commands/create-plugin/createPlugin.ts | 6 +- packages/cli/src/lib/tasks.ts | 108 +----------------- 3 files changed, 5 insertions(+), 113 deletions(-) diff --git a/packages/cli/src/commands/create-app/createApp.ts b/packages/cli/src/commands/create-app/createApp.ts index eed078342f..4f4134957d 100644 --- a/packages/cli/src/commands/create-app/createApp.ts +++ b/packages/cli/src/commands/create-app/createApp.ts @@ -22,7 +22,7 @@ import inquirer, { Answers, Question } from 'inquirer'; import { exec as execCb } from 'child_process'; import { resolve as resolvePath } from 'path'; import os from 'os'; -import { Task, templatingTask, installWithLocalDeps } from '../../lib/tasks'; +import { Task, templatingTask } from '../../lib/tasks'; import { paths } from '../../lib/paths'; import { version } from '../../lib/version'; @@ -72,7 +72,7 @@ async function buildApp(appDir: string) { }); }; - await installWithLocalDeps(appDir); + await runCmd('yarn install'); await runCmd('yarn tsc'); await runCmd('yarn build'); } diff --git a/packages/cli/src/commands/create-plugin/createPlugin.ts b/packages/cli/src/commands/create-plugin/createPlugin.ts index 07f68165a5..1898285a22 100644 --- a/packages/cli/src/commands/create-plugin/createPlugin.ts +++ b/packages/cli/src/commands/create-plugin/createPlugin.ts @@ -28,7 +28,7 @@ import { } from '../../lib/codeowners'; import { paths } from '../../lib/paths'; import { version } from '../../lib/version'; -import { Task, templatingTask, installWithLocalDeps } from '../../lib/tasks'; +import { Task, templatingTask } from '../../lib/tasks'; const exec = promisify(execCb); @@ -142,9 +142,7 @@ async function cleanUp(tempDir: string) { } async function buildPlugin(pluginFolder: string) { - await installWithLocalDeps(paths.targetRoot); - - const commands = ['yarn tsc', 'yarn build']; + const commands = ['yarn install', 'yarn tsc', 'yarn build']; for (const command of commands) { await Task.forItem('executing', command, async () => { process.chdir(pluginFolder); diff --git a/packages/cli/src/lib/tasks.ts b/packages/cli/src/lib/tasks.ts index f4cb31fd69..0753301b78 100644 --- a/packages/cli/src/lib/tasks.ts +++ b/packages/cli/src/lib/tasks.ts @@ -18,13 +18,8 @@ import chalk from 'chalk'; import fs from 'fs-extra'; import handlebars from 'handlebars'; import ora from 'ora'; -import { resolve as resolvePath, basename, dirname } from 'path'; +import { basename, dirname } from 'path'; import recursive from 'recursive-readdir'; -import { promisify } from 'util'; -import { exec as execCb } from 'child_process'; -import { paths } from './paths'; - -const exec = promisify(execCb); const TASK_NAME_MAX_LENGTH = 14; @@ -108,104 +103,3 @@ export async function templatingTask( } } } - -// List of local packages that we need to modify as a part of an E2E test -const PATCH_PACKAGES = [ - 'cli', - 'config', - 'config-loader', - 'core', - 'core-api', - 'dev-utils', - 'test-utils', - 'test-utils-core', - 'theme', -]; - -// This runs a `yarn install` task, but with special treatment for e2e tests -export async function installWithLocalDeps(dir: string) { - // This makes us install any package inside this repo as a local file dependency. - // For example, instead of trying to fetch @backstage/core from npm, we point it - // to /packages/core. This makes yarn use a simple file copy to install it instead. - if (process.env.BACKSTAGE_E2E_CLI_TEST) { - Task.section('Linking packages locally for e2e tests'); - - const pkgJsonPath = resolvePath(dir, 'package.json'); - const pkgJson = await fs.readJson(pkgJsonPath); - - pkgJson.resolutions = pkgJson.resolutions || {}; - pkgJson.dependencies = pkgJson.dependencies || {}; - - if (!pkgJson.resolutions[`@backstage/${PATCH_PACKAGES[0]}`]) { - for (const name of PATCH_PACKAGES) { - await Task.forItem( - 'adding', - `@backstage/${name} link to package.json`, - async () => { - const pkgPath = paths.resolveOwnRoot('packages', name); - // Add to both resolutions and dependencies, or transitive dependencies will still be fetched from the registry. - pkgJson.dependencies[`@backstage/${name}`] = `file:${pkgPath}`; - pkgJson.resolutions[`@backstage/${name}`] = `file:${pkgPath}`; - delete pkgJson.devDependencies[`@backstage/${name}`]; - - await fs - .writeJSON(pkgJsonPath, pkgJson, { encoding: 'utf8', spaces: 2 }) - .catch(error => { - throw new Error( - `Failed to add resolutions to package.json: ${error.message}`, - ); - }); - }, - ); - } - } - } - - await Task.forItem('executing', 'yarn install', async () => { - await exec('yarn install', { cwd: dir }).catch(error => { - process.stdout.write(error.stderr); - process.stdout.write(error.stdout); - throw new Error( - `Could not execute command ${chalk.cyan('yarn install')}`, - ); - }); - }); - - // This takes care of pointing all the installed packages from this repo to - // dist instead of the local src, using the field overrides in publishConfig. - // Without this we get type checking errors in the e2e test - if (process.env.BACKSTAGE_E2E_CLI_TEST) { - Task.section('Patching local dependencies for e2e tests'); - - for (const name of PATCH_PACKAGES) { - await Task.forItem( - 'patching', - `node_modules/@backstage/${name} package.json`, - async () => { - const depJsonPath = resolvePath( - dir, - 'node_modules/@backstage', - name, - 'package.json', - ); - const depJson = await fs.readJson(depJsonPath); - - // We want dist to be used for e2e tests - for (const key of Object.keys(depJson.publishConfig)) { - if (key !== 'access') { - depJson[key] = depJson.publishConfig[key]; - } - } - - await fs - .writeJSON(depJsonPath, depJson, { encoding: 'utf8', spaces: 2 }) - .catch(error => { - throw new Error( - `Failed to add resolutions to package.json: ${error.message}`, - ); - }); - }, - ); - } - } -} From 192868ffefef163a347529f2c6bdc0fe7e4443e8 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 2 Jul 2020 01:20:07 +0200 Subject: [PATCH 05/10] github/workflows: remove redundant cli checks --- .github/workflows/cli-win.yml | 18 ------------------ .github/workflows/cli.yml | 18 ------------------ 2 files changed, 36 deletions(-) diff --git a/.github/workflows/cli-win.yml b/.github/workflows/cli-win.yml index 00c7c26a6c..3e93c7a6d9 100644 --- a/.github/workflows/cli-win.yml +++ b/.github/workflows/cli-win.yml @@ -45,21 +45,3 @@ jobs: - name: verify app and plugin creation working-directory: ${{ runner.temp }} run: node ${{ github.workspace }}/packages/cli/e2e-test/cli-e2e-test.js - env: - BACKSTAGE_E2E_CLI_TEST: true - - name: lint newly created app and plugin - run: yarn lint:all - 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: ${{ runner.temp }}/test-app - env: - BACKSTAGE_E2E_CLI_TEST: true - - name: e2e test newly created app - run: yarn test:e2e:ci - working-directory: ${{ runner.temp }}/test-app/packages/app - env: - APP_CONFIG_app_baseUrl: '"http://localhost:3001"' - BACKSTAGE_E2E_CLI_TEST: true diff --git a/.github/workflows/cli.yml b/.github/workflows/cli.yml index 71b207c0d3..2f6ec865d1 100644 --- a/.github/workflows/cli.yml +++ b/.github/workflows/cli.yml @@ -48,21 +48,3 @@ jobs: run: | sudo sysctl fs.inotify.max_user_watches=524288 node ${{ github.workspace }}/packages/cli/e2e-test/cli-e2e-test.js - env: - BACKSTAGE_E2E_CLI_TEST: true - - name: lint newly created app and plugin - run: yarn lint:all - 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: ${{ runner.temp }}/test-app - env: - BACKSTAGE_E2E_CLI_TEST: true - - name: e2e test newly created app - run: yarn test:e2e:ci - working-directory: ${{ runner.temp }}/test-app/packages/app - env: - APP_CONFIG_app_baseUrl: '"http://localhost:3001"' - BACKSTAGE_E2E_CLI_TEST: true From dd26e15fd4e095eaefaa8999e584b32b82954eb8 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 2 Jul 2020 01:31:23 +0200 Subject: [PATCH 06/10] github/workflows: use correct working directory for e2e test --- .github/workflows/cli-win.yml | 1 - .github/workflows/cli.yml | 1 - 2 files changed, 2 deletions(-) diff --git a/.github/workflows/cli-win.yml b/.github/workflows/cli-win.yml index 3e93c7a6d9..1355ed251e 100644 --- a/.github/workflows/cli-win.yml +++ b/.github/workflows/cli-win.yml @@ -43,5 +43,4 @@ jobs: - run: yarn tsc - run: yarn build - name: verify app and plugin creation - working-directory: ${{ runner.temp }} run: node ${{ github.workspace }}/packages/cli/e2e-test/cli-e2e-test.js diff --git a/.github/workflows/cli.yml b/.github/workflows/cli.yml index 2f6ec865d1..7704aa36cd 100644 --- a/.github/workflows/cli.yml +++ b/.github/workflows/cli.yml @@ -44,7 +44,6 @@ jobs: - run: yarn tsc - run: yarn build - name: verify app and plugin creation - working-directory: ${{ runner.temp }} run: | sudo sysctl fs.inotify.max_user_watches=524288 node ${{ github.workspace }}/packages/cli/e2e-test/cli-e2e-test.js From 2aca5bb6d45954966e29a87c95ffc90e458eead6 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 2 Jul 2020 09:56:17 +0200 Subject: [PATCH 07/10] cli: refactor e2e-test to use runPlain with fix for windows --- packages/cli/e2e-test/cli-e2e-test.js | 15 +++++++-------- packages/cli/e2e-test/helpers.js | 21 +++++++++++++++++++-- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/packages/cli/e2e-test/cli-e2e-test.js b/packages/cli/e2e-test/cli-e2e-test.js index ff0e9193b0..b238c726a9 100644 --- a/packages/cli/e2e-test/cli-e2e-test.js +++ b/packages/cli/e2e-test/cli-e2e-test.js @@ -18,17 +18,15 @@ const os = require('os'); const fs = require('fs-extra'); const { resolve: resolvePath, join: joinPath } = require('path'); const Browser = require('zombie'); -const { execFile: execFileCb } = require('child_process'); -const { promisify } = require('util'); const { spawnPiped, + runPlain, handleError, waitForPageWithText, waitFor, waitForExit, print, } = require('./helpers'); -const execFile = promisify(execFileCb); async function main() { const rootDir = await fs.mkdtemp(resolvePath(os.tmpdir(), 'backstage-e2e-')); @@ -58,7 +56,8 @@ async function buildDistWorkspace(workspaceName, rootDir) { await fs.ensureDir(workspaceDir); print(`Preparing workspace`); - await execFile('yarn', [ + await runPlain([ + 'yarn', 'backstage-cli', 'build-workspace', workspaceDir, @@ -72,7 +71,7 @@ async function buildDistWorkspace(workspaceName, rootDir) { await pinYarnVersion(workspaceDir); print('Installing workspace dependencies'); - await execFile('yarn', ['install', '--production', '--frozen-lockfile'], { + await runPlain(['yarn', 'install', '--production', '--frozen-lockfile'], { cwd: workspaceDir, }); @@ -137,11 +136,11 @@ async function createApp(appName, workspaceDir, rootDir) { for (const cmd of ['install', 'tsc', 'build', 'lint:all', 'test:all']) { print(`Running 'yarn ${cmd}' in newly created app`); - await execFile('yarn', [cmd], { cwd: appDir }); + await runPlain(['yarn', cmd], { cwd: appDir }); } print(`Running 'yarn test:e2e:ci' in newly created app`); - await execFile('yarn', ['test:e2e:ci'], { + await runPlain(['yarn', 'test:e2e:ci'], { cwd: resolvePath(appDir, 'packages', 'app'), env: { ...process.env, @@ -202,7 +201,7 @@ async function createPlugin(pluginName, appDir) { const pluginDir = resolvePath(appDir, 'plugins', pluginName); for (const cmd of [['lint'], ['test', '--no-watch']]) { print(`Running 'yarn ${cmd.join(' ')}' in newly created plugin`); - await execFile('yarn', cmd, { cwd: pluginDir }); + await runPlain(['yarn', ...cmd], { cwd: pluginDir }); } return pluginName; diff --git a/packages/cli/e2e-test/helpers.js b/packages/cli/e2e-test/helpers.js index 1292ba19bf..4f5152e0b1 100644 --- a/packages/cli/e2e-test/helpers.js +++ b/packages/cli/e2e-test/helpers.js @@ -14,9 +14,10 @@ * limitations under the License. */ -const childProcess = require('child_process'); +const { spawn, execFile: execFileCb } = require('child_process'); +const { promisify } = require('util'); -const { spawn } = childProcess; +const execFile = promisify(execFileCb); const EXPECTED_LOAD_ERRORS = /ECONNREFUSED|ECONNRESET|did not get to load all resources/; @@ -57,6 +58,21 @@ function spawnPiped(cmd, options) { return child; } +async function runPlain(cmd, options) { + try { + const { stdout } = await execFile(cmd[0], cmd.slice(1), { + ...options, + shell: true, + }); + return stdout.trim(); + } catch (error) { + if (error.stderr) { + process.stderr.write(error.stderr); + } + throw error; + } +} + function handleError(err) { process.stdout.write(`${err.name}: ${err.stack || err.message}\n`); if (typeof err.code === 'number') { @@ -150,6 +166,7 @@ function print(msg) { module.exports = { spawnPiped, + runPlain, handleError, waitFor, waitForExit, From 7cd04c77fac7d830c03699739f578e99135d9740 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 2 Jul 2020 12:53:30 +0200 Subject: [PATCH 08/10] cli: use node to execude backstage-cli bin in e2e test --- packages/cli/e2e-test/cli-e2e-test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/cli/e2e-test/cli-e2e-test.js b/packages/cli/e2e-test/cli-e2e-test.js index b238c726a9..67cb71802f 100644 --- a/packages/cli/e2e-test/cli-e2e-test.js +++ b/packages/cli/e2e-test/cli-e2e-test.js @@ -99,6 +99,7 @@ async function pinYarnVersion(dir) { async function createApp(appName, workspaceDir, rootDir) { const child = spawnPiped( [ + 'node', resolvePath(workspaceDir, 'packages/cli/bin/backstage-cli'), 'create-app', '--skip-install', From c499656a690079eaed00a58cbbc2e565666fd6c8 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 2 Jul 2020 14:41:18 +0200 Subject: [PATCH 09/10] cli: use tree-kill to kill serve task in e2e test --- packages/cli/e2e-test/cli-e2e-test.js | 4 ++-- packages/cli/package.json | 1 + yarn.lock | 5 +++++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/cli/e2e-test/cli-e2e-test.js b/packages/cli/e2e-test/cli-e2e-test.js index 67cb71802f..e6a8b2e201 100644 --- a/packages/cli/e2e-test/cli-e2e-test.js +++ b/packages/cli/e2e-test/cli-e2e-test.js @@ -16,6 +16,7 @@ const os = require('os'); const fs = require('fs-extra'); +const killTree = require('tree-kill'); const { resolve: resolvePath, join: joinPath } = require('path'); const Browser = require('zombie'); const { @@ -217,7 +218,6 @@ async function createPlugin(pluginName, appDir) { async function testAppServe(pluginName, appDir) { const startApp = spawnPiped(['yarn', 'start'], { cwd: appDir, - detached: true, }); Browser.localhost('localhost', 3000); @@ -236,7 +236,7 @@ async function testAppServe(pluginName, appDir) { throw new Error(`App serve test failed, ${error}`); } finally { // Kill entire process group, otherwise we'll end up with hanging serve processes - process.kill(-startApp.pid, 'SIGTERM'); + killTree(startApp.pid); } await waitForExit(startApp); diff --git a/packages/cli/package.json b/packages/cli/package.json index f74c1b1fb0..1959e9cc07 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -112,6 +112,7 @@ "@types/webpack-dev-server": "^3.10.0", "del": "^5.1.0", "nodemon": "^2.0.2", + "tree-kill": "^1.2.2", "ts-node": "^8.6.2", "zombie": "^6.1.4" }, diff --git a/yarn.lock b/yarn.lock index adc4a02cd4..7a9b6c0bd5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -17999,6 +17999,11 @@ tr46@^2.0.2: dependencies: punycode "^2.1.1" +tree-kill@^1.2.2: + version "1.2.2" + resolved "https://registry.npmjs.org/tree-kill/-/tree-kill-1.2.2.tgz#4ca09a9092c88b73a7cdc5e8a01b507b0790a0cc" + integrity sha512-L0Orpi8qGpRG//Nd+H90vFB+3iHnue1zSSGmNOOCh1GLJ7rUKVwV2HvijphGQS2UmhUZewS9VgvxYIdgr+fG1A== + trim-newlines@^1.0.0: version "1.0.0" resolved "https://registry.npmjs.org/trim-newlines/-/trim-newlines-1.0.0.tgz#5887966bb582a4503a41eb524f7d35011815a613" From ab6385cfb1266a6c873a860a6db0fd4a7c51d2a7 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 2 Jul 2020 19:00:45 +0200 Subject: [PATCH 10/10] cli: never treat success as a failure in e2e test --- packages/cli/e2e-test/cli-e2e-test.js | 10 +++++++++- packages/cli/e2e-test/helpers.js | 6 ------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/cli/e2e-test/cli-e2e-test.js b/packages/cli/e2e-test/cli-e2e-test.js index e6a8b2e201..ef5d6ab243 100644 --- a/packages/cli/e2e-test/cli-e2e-test.js +++ b/packages/cli/e2e-test/cli-e2e-test.js @@ -221,6 +221,7 @@ async function testAppServe(pluginName, appDir) { }); Browser.localhost('localhost', 3000); + let successful = false; try { const browser = new Browser(); @@ -232,6 +233,7 @@ async function testAppServe(pluginName, appDir) { ); print('Both App and Plugin loaded correctly'); + successful = true; } catch (error) { throw new Error(`App serve test failed, ${error}`); } finally { @@ -239,7 +241,13 @@ async function testAppServe(pluginName, appDir) { killTree(startApp.pid); } - await waitForExit(startApp); + try { + await waitForExit(startApp); + } catch (error) { + if (!successful) { + throw error; + } + } } process.on('unhandledRejection', handleError); diff --git a/packages/cli/e2e-test/helpers.js b/packages/cli/e2e-test/helpers.js index 4f5152e0b1..da2ec343ab 100644 --- a/packages/cli/e2e-test/helpers.js +++ b/packages/cli/e2e-test/helpers.js @@ -38,12 +38,6 @@ function spawnPiped(cmd, options) { ...options, }); child.on('error', handleError); - child.on('exit', code => { - if (code) { - print(`Child '${cmd.join(' ')}' exited with code ${code}`); - process.exit(code); - } - }); const logPrefix = cmd.map(s => s.replace(/.+\//, '')).join(' '); child.stdout.on(