Merge branch 'master' into remove_plugin_wip

.github/workflows/cli.yml

@@ -41,31 +41,27 @@ 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
+        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
-        working-directory: ${{ steps.generate_tempdir.outputs.tempdir }}
+        working-directory: ${{ runner.temp }}
         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
-      # This should lint and test both an app and a plugin
-      - name: yarn lint, test after creation
-        working-directory: ${{ steps.generate_tempdir.outputs.tempdir }}
-        run: |
-          cd test-app
-          yarn lint:all
-          yarn test:all
+      - 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

package.json

@@ -34,8 +34,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",

packages/cli/e2e-test/.eslintrc.js

@@ -14,17 +14,16 @@
  * limitations under the License.
  */
 
-const { handleError } = require('./helpers');
-
-async function generateTempDir() {
-  const tempDir = await require('fs-extra').mkdtemp(
-    require('path').join(require('os').tmpdir(), 'backstage-e2e-'),
-  );
-  process.stdout.write(tempDir);
-  return tempDir;
-}
-
-module.exports = generateTempDir;
-
-process.on('unhandledRejection', handleError);
-generateTempDir().catch(handleError);
+module.exports = {
+  rules: {
+    'import/no-extraneous-dependencies': [
+      'error',
+      {
+        devDependencies: true,
+        optionalDependencies: true,
+        peerDependencies: true,
+        bundledDependencies: true,
+      },
+    ],
+  },
+};

packages/cli/e2e-test/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,35 +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 rootDir = process.env.CI
-    ? resolvePath(process.env.GITHUB_WORKSPACE)
-    : resolvePath(__dirname, '..');
-
-  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 waitForExit(spawnPiped(['yarn', 'init --yes']));
+  await createTestApp();
 
-  const createCmdPath = require('path').join(
-    rootDir,
-    'packages',
-    'cli',
-    'bin',
-    'backstage-cli',
-  );
-  await createTestApp(`${createCmdPath} create-app`);
-
-  const appDir = resolvePath(tempDir, 'test-app');
+  const appDir = resolvePath(workDir, 'test-app');
   process.chdir(appDir);
   process.stdout.write(`App directory: ${appDir}\n`);
 

packages/cli/e2e-test/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, '../bin/backstage-cli');
+
   print('Creating a Backstage App');
-  const createApp = spawnPiped(['node', cmd]);
+  const createApp = spawnPiped(['node', cliPath, 'create-app']);
 
   try {
     let stdout = '';

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"

packages/cli/src/commands/build-cache/index.ts

@@ -30,7 +30,7 @@ export async function withCache(
   buildFunc: () => Promise<void>,
 ): Promise<void> {
   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();

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');

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),
   };

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": {

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,
 };
 

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,
 };
 

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 = () => (
+  <div style={{ width }}>
+    <TrendLine data={[0.1, 0.7, 0.5, 0.8]} title="Trend over time" />
+  </div>
+);
+
+export const TrendingUp = () => (
+  <div style={{ width }}>
+    <TrendLine data={[0.1, 0.5, 0.9, 1.0]} title="Trend over time" />
+  </div>
+);
+
+export const TrendingDown = () => (
+  <div style={{ width }}>
+    <TrendLine data={[0.8, 0.7, 0.5, 0.1]} title="Trend over time" />
+  </div>
+);

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(<CategoryTrendline data={[]} title="sparkline" />),
+        wrapInThemedTestApp(<TrendLine data={[]} title="sparkline" />),
       );
       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(
-          <CategoryTrendline data={[0.5]} title="sparkline" />,
-        ),
+        wrapInThemedTestApp(<TrendLine data={[0.5]} title="sparkline" />),
       );
       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(
-          <CategoryTrendline data={[0.5, 0.95]} title="sparkline" />,
-        ),
+        wrapInThemedTestApp(<TrendLine data={[0.5, 0.95]} title="sparkline" />),
       );
       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(
-          <CategoryTrendline data={[0.5, 0.65]} title="sparkline" />,
-        ),
+        wrapInThemedTestApp(<TrendLine data={[0.5, 0.65]} title="sparkline" />),
       );
       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(
-          <CategoryTrendline data={[0.5, 0.4]} title="sparkline" />,
-        ),
+        wrapInThemedTestApp(<TrendLine data={[0.5, 0.4]} title="sparkline" />),
       );
       expect(rendered.getByTitle('sparkline')).toBeInTheDocument();
     });

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<SparklinesProps & { title?: string }> = props => {
+const Trendline: FC<SparklinesProps & { title?: string }> = props => {
   const theme = useTheme<BackstageTheme>();
 
   if (!props.data) return null;
@@ -38,4 +39,4 @@ const CategoryTrendline: FC<SparklinesProps & { title?: string }> = props => {
   );
 };
 
-export default CategoryTrendline;
+export default Trendline;

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';

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';

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",

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}
                 >
-                  <CategoryTrendline
+                  <TrendLine
                     title={`trendline for ${CATEGORY_LABELS[category]} category of ${website.url}`}
                     data={categorySparklines[website.url][category] || []}
                   />