-
-
Notifications
You must be signed in to change notification settings - Fork 125
Attempt at speeding up prisma-generator tests #1465
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
Changes from 4 commits
3d342f3
408f710
bcee249
11068cb
208d1a9
63db458
f694528
96a92a3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,3 +8,4 @@ dist | |
| coverage | ||
| .build | ||
| .test | ||
| /.pnpm-test-store | ||
| 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-')); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
WimTibackx marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
WimTibackx marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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')} | |
| } | |
| }`; | |
| // 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}); | |
| } | |
| } |
Outdated
There was a problem hiding this comment.
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.
| 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; | |
| } | |
| } |
Uh oh!
There was an error while loading. Please reload this page.