chore(release): Fix hang in the test suite (#3005)

* Check for file existence before `npm install` on deploy, this was hanging up in some test environments
* Prefer absolute paths on the deploy pipeline
* Fix `ng add` stalling if you don't select any options

Author: jamesdaniels

package.json

@@ -16,7 +16,7 @@
     "test:typings": "node ./tools/run-typings-test.js",
     "test:build": "bash ./test/ng-build/build.sh",
     "test:all": "npm run test:node && npm run test:chrome-headless && npm run test:typings && npm run test:build",
-    "build": "rimraf dist && ttsc -p tsconfig.build.json && node --trace-warnings ./tools/build.js",
+    "build": "rimraf dist && ttsc -p tsconfig.build.json && node --trace-warnings ./tools/build.js && npm pack ./dist/packages-dist",
     "build:jasmine": "tsc -p tsconfig.jasmine.json && cp ./dist/packages-dist/schematics/versions.json ./dist/out-tsc/jasmine/schematics",
     "changelog": "conventional-changelog -p angular -i CHANGELOG.md -s -r 1"
   },

src/schematics/deploy/actions.jasmine.ts

@@ -26,6 +26,8 @@ login.list = () => Promise.resolve([{ user: { email: '[email protected]' }}]);
 login.add = () => Promise.resolve([{ user: { email: '[email protected]' }}]);
 login.use = () => Promise.resolve('[email protected]');
 
+const workspaceRoot = join('home', 'user');
+
 const initMocks = () => {
   fsHost = {
     moveSync(_: string, __: string) {
@@ -37,7 +39,10 @@ const initMocks = () => {
     copySync(_: string, __: string) {
     },
     removeSync(_: string) {
-    }
+    },
+    existsSync(_: string) {
+      return false;
+    },
   };
 
   firebaseMock = {
@@ -183,7 +188,7 @@ describe('universal deployment', () => {
     await deployToFunction(
       firebaseMock,
       context,
-      '/home/user',
+      workspaceRoot,
       STATIC_BUILD_TARGET,
       SERVER_BUILD_TARGET,
       { preview: false  },
@@ -196,16 +201,16 @@ describe('universal deployment', () => {
     const packageArgs = spy.calls.argsFor(0);
     const functionArgs = spy.calls.argsFor(1);
 
-    expect(packageArgs[0]).toBe(join('dist', 'package.json'));
-    expect(functionArgs[0]).toBe(join('dist', 'index.js'));
+    expect(packageArgs[0]).toBe(join(workspaceRoot, 'dist', 'package.json'));
+    expect(functionArgs[0]).toBe(join(workspaceRoot, 'dist', 'index.js'));
   });
 
   it('should create a firebase function (new)', async () => {
     const spy = spyOn(fsHost, 'writeFileSync');
     await deployToFunction(
       firebaseMock,
       context,
-      '/home/user',
+      workspaceRoot,
       STATIC_BUILD_TARGET,
       SERVER_BUILD_TARGET,
       { preview: false, outputPath: join('dist', 'functions') },
@@ -218,16 +223,16 @@ describe('universal deployment', () => {
     const packageArgs = spy.calls.argsFor(0);
     const functionArgs = spy.calls.argsFor(1);
 
-    expect(packageArgs[0]).toBe(join('dist', 'functions', 'package.json'));
-    expect(functionArgs[0]).toBe(join('dist', 'functions', 'index.js'));
+    expect(packageArgs[0]).toBe(join(workspaceRoot, 'dist', 'functions', 'package.json'));
+    expect(functionArgs[0]).toBe(join(workspaceRoot, 'dist', 'functions', 'index.js'));
   });
 
   it('should rename the index.html file in the nested dist', async () => {
     const spy = spyOn(fsHost, 'renameSync');
     await deployToFunction(
       firebaseMock,
       context,
-      '/home/user',
+      workspaceRoot,
       STATIC_BUILD_TARGET,
       SERVER_BUILD_TARGET,
       { preview: false  },
@@ -240,8 +245,8 @@ describe('universal deployment', () => {
     const packageArgs = spy.calls.argsFor(0);
 
     expect(packageArgs).toEqual([
-      join('dist', 'dist', 'browser', 'index.html'),
-      join('dist', 'dist', 'browser', 'index.original.html')
+      join(workspaceRoot, 'dist', 'dist', 'browser', 'index.html'),
+      join(workspaceRoot, 'dist', 'dist', 'browser', 'index.original.html')
     ]);
   });
 
@@ -250,7 +255,7 @@ describe('universal deployment', () => {
     await deployToFunction(
       firebaseMock,
       context,
-      '/home/user',
+      workspaceRoot,
       STATIC_BUILD_TARGET,
       SERVER_BUILD_TARGET,
       { preview: false, outputPath: join('dist', 'functions') },
@@ -263,8 +268,8 @@ describe('universal deployment', () => {
     const packageArgs = spy.calls.argsFor(0);
 
     expect(packageArgs).toEqual([
-      join('dist', 'functions', 'dist', 'browser', 'index.html'),
-      join('dist', 'functions', 'dist', 'browser', 'index.original.html')
+      join(workspaceRoot, 'dist', 'functions', 'dist', 'browser', 'index.html'),
+      join(workspaceRoot, 'dist', 'functions', 'dist', 'browser', 'index.original.html')
     ]);
   });
 
@@ -273,7 +278,7 @@ describe('universal deployment', () => {
     await deployToFunction(
       firebaseMock,
       context,
-      '/home/user',
+      workspaceRoot,
       STATIC_BUILD_TARGET,
       SERVER_BUILD_TARGET,
       { preview: false },

src/schematics/deploy/actions.ts

@@ -110,6 +110,7 @@ const defaultFsHost: FSHost = {
   renameSync,
   copySync,
   removeSync,
+  existsSync,
 };
 
 const findPackageVersion = (packageManager: string, name: string) => {
@@ -176,14 +177,14 @@ export const deployToFunction = async (
     );
   }
 
-  const staticOut = staticBuildOptions.outputPath;
-  const serverOut = serverBuildOptions.outputPath;
+  const staticOut = join(workspaceRoot, staticBuildOptions.outputPath);
+  const serverOut = join(workspaceRoot, serverBuildOptions.outputPath);
 
-  const functionsOut = options.outputPath || dirname(serverOut);
+  const functionsOut = options.outputPath ? join(workspaceRoot, options.outputPath) : dirname(serverOut);
   const functionName = options.functionName || DEFAULT_FUNCTION_NAME;
 
-  const newStaticOut = join(functionsOut, staticOut);
-  const newServerOut = join(functionsOut, serverOut);
+  const newStaticOut = join(functionsOut, staticBuildOptions.outputPath);
+  const newServerOut = join(functionsOut, serverBuildOptions.outputPath);
 
   // New behavior vs. old
   if (options.outputPath) {
@@ -204,14 +205,15 @@ export const deployToFunction = async (
     );
   }
 
+  const functionsPackageJsonPath = join(functionsOut, 'package.json');
   fsHost.writeFileSync(
-    join(functionsOut, 'package.json'),
+    functionsPackageJsonPath,
     JSON.stringify(packageJson, null, 2)
   );
 
   fsHost.writeFileSync(
     join(functionsOut, 'index.js'),
-    defaultFunction(serverOut, options, functionName)
+    defaultFunction(serverBuildOptions.outputPath, options, functionName)
   );
 
   if (!options.prerender) {
@@ -226,10 +228,10 @@ export const deployToFunction = async (
   // tslint:disable-next-line:no-non-null-assertion
   const siteTarget = options.target ?? context.target!.project;
 
-  try {
-    execSync(`npm --prefix ${functionsOut} i`);
-  } catch (e) {
-    console.warn(e.messsage);
+  if (fsHost.existsSync(functionsPackageJsonPath)) {
+    execSync(`npm --prefix ${functionsOut} install`);
+  } else {
+    console.error(`No package.json exists at ${functionsOut}`);
   }
 
   if (options.preview) {
@@ -287,15 +289,14 @@ export const deployToCloudRun = async (
     );
   }
 
-  const staticOut = staticBuildOptions.outputPath;
-  const serverOut = serverBuildOptions.outputPath;
+  const staticOut = join(workspaceRoot, staticBuildOptions.outputPath);
+  const serverOut = join(workspaceRoot, serverBuildOptions.outputPath);
 
-  // TODO pull these from firebase config
-  const cloudRunOut = options.outputPath || staticBuildOptions.outputPath.replace('/browser', '/run');
+  const cloudRunOut = options.outputPath ? join(workspaceRoot, options.outputPath) : join(dirname(serverOut), 'run');
   const serviceId = options.functionName || DEFAULT_FUNCTION_NAME;
 
-  const newStaticOut = join(cloudRunOut, staticOut);
-  const newServerOut = join(cloudRunOut, serverOut);
+  const newStaticOut = join(cloudRunOut, staticBuildOptions.outputPath);
+  const newServerOut = join(cloudRunOut, serverBuildOptions.outputPath);
 
   // This is needed because in the server output there's a hardcoded dependency on $cwd/dist/browser,
   // This assumes that we've deployed our application dist directory and we're running the server
@@ -305,7 +306,7 @@ export const deployToCloudRun = async (
   fsHost.copySync(staticOut, newStaticOut);
   fsHost.copySync(serverOut, newServerOut);
 
-  const packageJson = getPackageJson(context, workspaceRoot, options, join(serverOut, 'main.js'));
+  const packageJson = getPackageJson(context, workspaceRoot, options, join(serverBuildOptions.outputPath, 'main.js'));
   const nodeVersion = packageJson.engines.node;
 
   if (!satisfies(process.versions.node, nodeVersion.toString())) {

src/schematics/interfaces.ts

@@ -201,6 +201,7 @@ export interface FSHost {
   renameSync(src: string, dest: string): void;
   copySync(src: string, dest: string): void;
   removeSync(src: string): void;
+  existsSync(src: string): boolean;
 }
 
 export interface WorkspaceProject {

src/schematics/setup/index.ts

@@ -105,6 +105,8 @@ ${Object.entries(config.sdkConfig).reduce(
           });
         default: throw(new SchematicsException(`Unimplemented PROJECT_TYPE ${config.projectType}`));
       }
+    } else {
+      return Promise.resolve();
     }
 };