Addressing PR feedback

cloudflare · Feb 6, 2024 · 0535c9e · 0535c9e
1 parent 35a1a7e
commit 0535c9e
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 41 deletions.
diff --git a/packages/create-cloudflare/e2e-tests/fixtures/qwik/wrangler.toml b/packages/create-cloudflare/e2e-tests/fixtures/qwik/wrangler.toml
@@ -1,2 +1,2 @@
 [vars]
-TEST = "C3_TEST"
+TEST = "C3_TEST"
diff --git a/packages/create-cloudflare/e2e-tests/frameworks.test.ts b/packages/create-cloudflare/e2e-tests/frameworks.test.ts
@@ -41,13 +41,13 @@ type FrameworkTestConfig = RunnerConfig & {
 	unsupportedOSs?: string[];
 	verifyDev?: {
 		route: string;
-		expectedToken: string;
+		expectedText: string;
 	};
 	verifyBuild?: {
 		outputDir: string;
 		script: string;
 		route: string;
-		expectedToken: string;
+		expectedText: string;
 	};
 };
 
@@ -58,7 +58,7 @@ const frameworkTests: Record<string, FrameworkTestConfig> = {
 		unsupportedOSs: ["win32"],
 		verifyDeploy: {
 			route: "/",
-			expectedToken: "Hello, Astronaut!",
+			expectedText: "Hello, Astronaut!",
 		},
 	},
 	docusaurus: {
@@ -68,15 +68,15 @@ const frameworkTests: Record<string, FrameworkTestConfig> = {
 		timeout: LONG_TIMEOUT,
 		verifyDeploy: {
 			route: "/",
-			expectedToken: "Dinosaurs are cool",
+			expectedText: "Dinosaurs are cool",
 		},
 	},
 	angular: {
 		testCommitMessage: true,
 		timeout: LONG_TIMEOUT,
 		verifyDeploy: {
 			route: "/",
-			expectedToken: "Congratulations! Your app is running.",
+			expectedText: "Congratulations! Your app is running.",
 		},
 	},
 	gatsby: {
@@ -91,14 +91,14 @@ const frameworkTests: Record<string, FrameworkTestConfig> = {
 		timeout: LONG_TIMEOUT,
 		verifyDeploy: {
 			route: "/",
-			expectedToken: "Gatsby!",
+			expectedText: "Gatsby!",
 		},
 	},
 	hono: {
 		testCommitMessage: false,
 		verifyDeploy: {
 			route: "/",
-			expectedToken: "Hello Hono!",
+			expectedText: "Hello Hono!",
 		},
 	},
 	qwik: {
@@ -113,17 +113,17 @@ const frameworkTests: Record<string, FrameworkTestConfig> = {
 		unsupportedPms: ["yarn"],
 		verifyDeploy: {
 			route: "/",
-			expectedToken: "Welcome to Qwik",
+			expectedText: "Welcome to Qwik",
 		},
 		verifyDev: {
 			route: "/test",
-			expectedToken: "C3_TEST",
+			expectedText: "C3_TEST",
 		},
 		verifyBuild: {
 			outputDir: "./dist",
 			script: "build",
 			route: "/test",
-			expectedToken: "C3_TEST",
+			expectedText: "C3_TEST",
 		},
 	},
 	remix: {
@@ -132,7 +132,7 @@ const frameworkTests: Record<string, FrameworkTestConfig> = {
 		unsupportedPms: ["yarn"],
 		verifyDeploy: {
 			route: "/",
-			expectedToken: "Welcome to Remix",
+			expectedText: "Welcome to Remix",
 		},
 	},
 	next: {
@@ -146,15 +146,15 @@ const frameworkTests: Record<string, FrameworkTestConfig> = {
 		quarantine: true,
 		verifyDeploy: {
 			route: "/",
-			expectedToken: "Create Next App",
+			expectedText: "Create Next App",
 		},
 	},
 	nuxt: {
 		testCommitMessage: true,
 		timeout: LONG_TIMEOUT,
 		verifyDeploy: {
 			route: "/",
-			expectedToken: "Welcome to Nuxt!",
+			expectedText: "Welcome to Nuxt!",
 		},
 	},
 	react: {
@@ -163,7 +163,7 @@ const frameworkTests: Record<string, FrameworkTestConfig> = {
 		timeout: LONG_TIMEOUT,
 		verifyDeploy: {
 			route: "/",
-			expectedToken: "React App",
+			expectedText: "React App",
 		},
 	},
 	solid: {
@@ -186,7 +186,7 @@ const frameworkTests: Record<string, FrameworkTestConfig> = {
 		unsupportedOSs: ["win32"],
 		verifyDeploy: {
 			route: "/",
-			expectedToken: "Hello world",
+			expectedText: "Hello world",
 		},
 	},
 	svelte: {
@@ -209,15 +209,15 @@ const frameworkTests: Record<string, FrameworkTestConfig> = {
 		unsupportedPms: ["npm"],
 		verifyDeploy: {
 			route: "/",
-			expectedToken: "SvelteKit app",
+			expectedText: "SvelteKit app",
 		},
 	},
 	vue: {
 		testCommitMessage: true,
 		unsupportedOSs: ["win32"],
 		verifyDeploy: {
 			route: "/",
-			expectedToken: "Vite App",
+			expectedText: "Vite App",
 		},
 	},
 };
@@ -308,7 +308,7 @@ describe.concurrent(`E2E: Web frameworks`, () => {
 					// Make a request to the deployed project and verify it was successful
 					await verifyDeployment(
 						`${deploymentUrl}${verifyDeploy.route}`,
-						verifyDeploy.expectedToken
+						verifyDeploy.expectedText
 					);
 
 					// Copy over any test fixture files
@@ -372,15 +372,15 @@ const runCli = async (
 
 const verifyDeployment = async (
 	deploymentUrl: string,
-	expectedToken: string
+	expectedText: string
 ) => {
 	await retry({ times: 5 }, async () => {
-		await new Promise((resolve) => setTimeout(resolve, 1000)); // wait a second
+		await sleep(1000);
 		const res = await fetch(deploymentUrl);
 		const body = await res.text();
-		if (!body.includes(expectedToken)) {
+		if (!body.includes(expectedText)) {
 			throw new Error(
-				`Deployed page (${deploymentUrl}) didn't contain expected string: "${expectedToken}"`
+				`Deployed page (${deploymentUrl}) didn't contain expected string: "${expectedText}"`
 			);
 		}
 	});
@@ -435,7 +435,7 @@ const verifyDevScript = async (
 	// end up camped and cause future runs to fail
 	await sleep(1000);
 
-	expect(body).toContain(verifyDev.expectedToken);
+	expect(body).toContain(verifyDev.expectedText);
 };
 
 const verifyBuildScript = async (
@@ -449,7 +449,7 @@ const verifyBuildScript = async (
 		return;
 	}
 
-	const { outputDir, script, route, expectedToken } = verifyBuild;
+	const { outputDir, script, route, expectedText } = verifyBuild;
 
 	// Run the build script
 	const { name: pm, npx } = detectPackageManager();
@@ -488,5 +488,5 @@ const verifyBuildScript = async (
 	await sleep(1000);
 
 	// Verify expectation after killing the process so that it exits cleanly in case of failure
-	expect(body).toContain(expectedToken);
+	expect(body).toContain(expectedText);
 };
diff --git a/packages/create-cloudflare/e2e-tests/helpers.ts b/packages/create-cloudflare/e2e-tests/helpers.ts
@@ -56,7 +56,7 @@ export type RunnerConfig = {
 	timeout?: number;
 	verifyDeploy?: {
 		route: string;
-		expectedToken: string;
+		expectedText: string;
 	};
 };
 

diff --git a/packages/create-cloudflare/e2e-tests/workers.test.ts b/packages/create-cloudflare/e2e-tests/workers.test.ts
@@ -1,5 +1,6 @@
 import { join } from "path";
 import { retry } from "helpers/command";
+import { sleep } from "helpers/common";
 import { readToml } from "helpers/files";
 import { fetch } from "undici";
 import { beforeAll, beforeEach, describe, expect, test } from "vitest";
@@ -28,14 +29,14 @@ const workerTemplates: WorkerTestConfig[] = [
 		template: "hello-world",
 		verifyDeploy: {
 			route: "/",
-			expectedToken: "Hello World!",
+			expectedText: "Hello World!",
 		},
 	},
 	{
 		template: "common",
 		verifyDeploy: {
 			route: "/",
-			expectedToken: "Try making requests to:",
+			expectedText: "Try making requests to:",
 		},
 	},
 	{
@@ -51,7 +52,7 @@ const workerTemplates: WorkerTestConfig[] = [
 		promptHandlers: [],
 		verifyDeploy: {
 			route: "/",
-			expectedToken: "SwaggerUI",
+			expectedText: "SwaggerUI",
 		},
 	},
 ];
@@ -132,7 +133,7 @@ describe
 
 							const { verifyDeploy } = template;
 							if (verifyDeploy && deployedUrl) {
-								await verifyDeployment(deployedUrl, verifyDeploy.expectedToken);
+								await verifyDeployment(deployedUrl, verifyDeploy.expectedText);
 							}
 						} finally {
 							clean(name);
@@ -185,7 +186,7 @@ const verifyDeployment = async (
 	expectedString: string
 ) => {
 	await retry({ times: 5 }, async () => {
-		await new Promise((resolve) => setTimeout(resolve, 1000)); // wait a second
+		await sleep(1000);
 		const res = await fetch(deploymentUrl);
 		const body = await res.text();
 		if (!body.includes(expectedString)) {

diff --git a/packages/create-cloudflare/src/templates.ts b/packages/create-cloudflare/src/templates.ts
@@ -368,34 +368,38 @@ const downloadRemoteTemplate = async (src: string) => {
 };
 
 export const updatePackageName = async (ctx: C3Context) => {
-	const s = spinner();
-	s.start("Updating name in `package.json`");
-
 	// Update package.json with project name
 	const placeholderNames = ["<TBD>", "TBD", ""];
 	const pkgJsonPath = resolve(ctx.project.path, "package.json");
 	const pkgJson = readJSON(pkgJsonPath);
 
-	if (placeholderNames.includes(pkgJson.name)) {
-		pkgJson.name = ctx.project.name;
+	if (!placeholderNames.includes(pkgJson.name)) {
+		return;
 	}
 
+	const s = spinner();
+	s.start("Updating name in `package.json`");
+
+	pkgJson.name = ctx.project.name;
+
 	writeJSON(pkgJsonPath, pkgJson);
 	s.stop(`${brandColor("updated")} ${dim("`package.json`")}`);
 };
 
 export const updatePackageScripts = async (ctx: C3Context) => {
+	if (!ctx.template.transformPackageJson) {
+		return;
+	}
+
 	const s = spinner();
 	s.start("Updating `package.json` scripts");
 
 	const pkgJsonPath = resolve(ctx.project.path, "package.json");
 	let pkgJson = readJSON(pkgJsonPath);
 
 	// Run any transformers defined by the template
-	if (ctx.template.transformPackageJson) {
-		const transformed = await ctx.template.transformPackageJson(pkgJson);
-		pkgJson = deepmerge(pkgJson, transformed);
-	}
+	const transformed = await ctx.template.transformPackageJson(pkgJson);
+	pkgJson = deepmerge(pkgJson, transformed);
 
 	writeJSON(pkgJsonPath, pkgJson);
 	s.stop(`${brandColor("updated")} ${dim("`package.json`")}`);