Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
33 changes: 19 additions & 14 deletions src/schematics/deploy/actions.jasmine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -37,7 +39,10 @@ const initMocks = () => {
copySync(_: string, __: string) {
},
removeSync(_: string) {
}
},
existsSync(_: string) {
return false;
},
};

firebaseMock = {
Expand Down Expand Up @@ -183,7 +188,7 @@ describe('universal deployment', () => {
await deployToFunction(
firebaseMock,
context,
'/home/user',
workspaceRoot,
STATIC_BUILD_TARGET,
SERVER_BUILD_TARGET,
{ preview: false },
Expand All @@ -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') },
Expand All @@ -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 },
Expand All @@ -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')
]);
});

Expand All @@ -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') },
Expand All @@ -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')
]);
});

Expand All @@ -273,7 +278,7 @@ describe('universal deployment', () => {
await deployToFunction(
firebaseMock,
context,
'/home/user',
workspaceRoot,
STATIC_BUILD_TARGET,
SERVER_BUILD_TARGET,
{ preview: false },
Expand Down
37 changes: 19 additions & 18 deletions src/schematics/deploy/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ const defaultFsHost: FSHost = {
renameSync,
copySync,
removeSync,
existsSync,
};

const findPackageVersion = (packageManager: string, name: string) => {
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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
Expand All @@ -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())) {
Expand Down
1 change: 1 addition & 0 deletions src/schematics/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions src/schematics/setup/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ ${Object.entries(config.sdkConfig).reduce(
});
default: throw(new SchematicsException(`Unimplemented PROJECT_TYPE ${config.projectType}`));
}
} else {
return Promise.resolve();
}
};

Expand Down