From 11068cbf5447e527754ca08f4e96d1110922426c Mon Sep 17 00:00:00 2001
From: Wim Tibackx <tibackx.wim@gmail.com>
Date: Sat, 1 Jun 2024 14:28:57 +0200
Subject: [PATCH] Improve error handling along code review comments and
 document design decisions

---
 .../tests/generator/prisma-generator.test.ts  |  2 +-
 script/test-utils.ts                          | 31 ++++++++++---------
 2 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/packages/schema/tests/generator/prisma-generator.test.ts b/packages/schema/tests/generator/prisma-generator.test.ts
index 1956e0d1a..fcd6ad12f 100644
--- a/packages/schema/tests/generator/prisma-generator.test.ts
+++ b/packages/schema/tests/generator/prisma-generator.test.ts
@@ -25,7 +25,7 @@ describe('Prisma generator test', () => {
     beforeEach(() => {
         origDir = process.cwd();
         const r = tmp.dirSync({ unsafeCleanup: true });
-        console.log(`Project dir: ${r.name}`);
+        console.log('Project dir: ', r.name);
         process.chdir(r.name);
 
         initProjectDir(r.name, packageJsonContents);
diff --git a/script/test-utils.ts b/script/test-utils.ts
index 5ad21356e..e3d86ec4d 100644
--- a/script/test-utils.ts
+++ b/script/test-utils.ts
@@ -10,7 +10,9 @@ export const PACKAGE_JSON_FILE = 'package.json';
 export const PACKAGE_JSON_CONTENTS = '{"name":"test-project","version":"1.0.0"}';
 
 export function preparePackageJson(dependencies: {[key: string]: string} = {}, devDependencies: {[key: string]: string} = {}): string {
+  // Given that this is a loose file included from elsewhere, I couldn't rely on the tmp package here and had to go with built-in node functions. I saw no significant downsides in this case, versus the upside in developer experience of not needing to do a build step when changing these utils.
   const tmpDir = fs.mkdtempSync(path.join(tmpdir(), 'zenstack-test-'));
+  console.log(`Loading dependencies into store via temp dir ${tmpDir}`);
   try {
   const packageJsonContents = 
 `{
@@ -24,30 +26,31 @@ export function preparePackageJson(dependencies: {[key: string]: string} = {}, d
   }
 }`;
 
+  // I considered doing a `pnpm store add` here instead of a plain install. While that worked, I decided against it in the end because it's a secondary way of processing the dependencies and I didn't see a significant downside to just installing and throwing the local project away right after.
   initProjectDir(tmpDir, packageJsonContents, false);
 
   return packageJsonContents;
   } finally {
     fs.rmSync(tmpDir, {recursive: true, force: true});
-    console.log(`Loaded dependencies into store via temp dir ${tmpDir}`);
   }
 }
 
-function execCmdSync(cmd: string, path: string) {
-  console.log(`Running: ${cmd}, in ${path}`);
+export function initProjectDir(projectDir: string, packageJsonContents: string, offline = true) {
   try {
-      execSync(cmd, { cwd: path, stdio: 'ignore' });
-  } catch (err) {
-      console.error(`Test project scaffolding cmd error: ${err}`);
-      throw err;
+    if (!fs.existsSync(projectDir)) {
+      fs.mkdirSync(projectDir, { recursive: true });
+    }
+    fs.writeFileSync(path.join(projectDir, PACKAGE_JSON_FILE), packageJsonContents, { flag: 'w+' });
+    fs.writeFileSync(path.join(projectDir, NPM_RC_FILE), NPM_RC_CONTENTS, { flag: 'w+' });
+  } catch (e) {
+    console.error(`Failed to set up project dir in ${projectDir}`);
+    throw e;
   }
-}
 
-export function initProjectDir(projectDir: string, packageJsonContents: string, offline = true) {
-  if (!fs.existsSync(projectDir)) {
-    fs.mkdirSync(projectDir, { recursive: true });
+  try {
+    execSync(`pnpm install ${offline ? '--offline ' : ''}--ignore-workspace`, {cwd: projectDir, stdio: 'ignore'});
+  } catch (e) {
+    console.error(`Failed to initialize project dependencies in ${projectDir}${offline ? '(offline mode)' : '(online mode)'}`);
+    throw e;
   }
-  fs.writeFileSync(path.join(projectDir, PACKAGE_JSON_FILE), packageJsonContents, { flag: 'w+' });
-  fs.writeFileSync(path.join(projectDir, NPM_RC_FILE), NPM_RC_CONTENTS, { flag: 'w+' });
-  execCmdSync(`pnpm install ${offline ? '--offline ' : ''}--ignore-workspace`, projectDir);
 }