Skip to content
Closed
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ dist
coverage
.build
.test
/.pnpm-test-store
15 changes: 11 additions & 4 deletions packages/schema/tests/generator/prisma-generator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,29 @@ import path from 'path';
import tmp from 'tmp';
import { loadDocument } from '../../src/cli/cli-util';
import { PrismaSchemaGenerator } from '../../src/plugins/prisma/schema-generator';
import { execSync } from '../../src/utils/exec-utils';
import { loadModel } from '../utils';
import { preparePackageJson, initProjectDir } from '../../../../script/test-utils';

tmp.setGracefulCleanup();

describe('Prisma generator test', () => {
let origDir: string;

let packageJsonContents: string;

beforeAll(async () => {
packageJsonContents = preparePackageJson({
'prisma': '^5.0.0'
});
})

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

execSync('npm init -y', { stdio: 'ignore' });
execSync('npm install prisma');
initProjectDir(r.name, packageJsonContents);
});

afterEach(() => {
Expand Down
56 changes: 56 additions & 0 deletions script/test-utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import path from 'node:path';
import fs from 'node:fs';
import { tmpdir } from 'node:os';
import { execSync } from 'node:child_process';

export const PNPM_STORE_PATH = path.resolve(__dirname, '../.pnpm-test-store');
export const NPM_RC_FILE = '.npmrc';
export const NPM_RC_CONTENTS = `store-dir = ${PNPM_STORE_PATH}`;
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 =
`{
"name":"test-project",
"version":"1.0.0",
"dependencies": {
${Object.entries(dependencies).map(([k, v]) => `"${k}": "${v}"`).join(',\n')}
},
"devDependencies": {
${Object.entries(devDependencies).map(([k, v]) => `"${k}": "${v}"`).join(',\n')}
}
}`;

// 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});
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The preparePackageJson function is well-structured for generating a temporary directory and handling dependencies. Consider adding a comment explaining the decision to use pnpm install instead of pnpm store add for future maintainers.

+ // Using `pnpm install` for simplicity and because it's the primary way of handling dependencies.
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
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 =
`{
"name":"test-project",
"version":"1.0.0",
"dependencies": {
${Object.entries(dependencies).map(([k, v]) => `"${k}": "${v}"`).join(',\n')}
},
"devDependencies": {
${Object.entries(devDependencies).map(([k, v]) => `"${k}": "${v}"`).join(',\n')}
}
}`;
// 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});
}
}
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 =
`{
"name":"test-project",
"version":"1.0.0",
"dependencies": {
${Object.entries(dependencies).map(([k, v]) => `"${k}": "${v}"`).join(',\n')}
},
"devDependencies": {
${Object.entries(devDependencies).map(([k, v]) => `"${k}": "${v}"`).join(',\n')}
}
}`;
// Using `pnpm install` for simplicity and because it's the primary way of handling dependencies.
// 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});
}
}


export function initProjectDir(projectDir: string, packageJsonContents: string, offline = true) {
try {
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;
}

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;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The initProjectDir function effectively sets up the project directory and handles potential errors robustly. However, the error messages could be made more descriptive to aid debugging.

- console.error(`Failed to set up project dir in ${projectDir}`);
+ console.error(`Failed to set up project directory in ${projectDir}: ${e.message}`);

- console.error(`Failed to initialize project dependencies in ${projectDir}${offline ? '(offline mode)' : '(online mode)'}`);
+ console.error(`Failed to initialize project dependencies in ${projectDir}${offline ? '(offline mode)' : '(online mode)'}: ${e.message}`);
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export function initProjectDir(projectDir: string, packageJsonContents: string, offline = true) {
try {
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;
}
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;
}
}
export function initProjectDir(projectDir: string, packageJsonContents: string, offline = true) {
try {
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 directory in ${projectDir}: ${e.message}`);
throw e;
}
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)'}: ${e.message}`);
throw e;
}
}