chore(PRISMA_FORCE_NAPI): comments and links (#7813)

prisma · Jun 24, 2021 · 5a4d80c · 5a4d80c
1 parent f10951f
commit 5a4d80c
Show file tree

Hide file tree

Showing 15 changed files with 60 additions and 27 deletions.
diff --git a/src/packages/cli/src/Version.ts b/src/packages/cli/src/Version.ts
@@ -71,6 +71,7 @@ export class Version implements Command {
       BinaryType.introspectionEngine,
     )
     const migrationEngine = await this.resolveEngine(BinaryType.migrationEngine)
+    // TODO This conditional does not really belong here, CLI should be able to tell you which engine it is _actually_ using
     const queryEngine = await this.resolveEngine(
       useNAPI ? BinaryType.libqueryEngineNapi : BinaryType.queryEngine,
     )

diff --git a/src/packages/cli/src/__tests__/studio.test.ts b/src/packages/cli/src/__tests__/studio.test.ts
@@ -21,7 +21,7 @@ let studio: Studio
 const describeIf = (condition: boolean) =>
   condition ? describe : describe.skip
 
-describeIf(process.env.PRISMA_FORCE_NAPI !== 'true')('studio', () => {
+describe('studio', () => {
   beforeAll(async () => {
     // Before every test, we'd like to reset the DB.
     // We do this by duplicating the original SQLite DB file, and using the duplicate as the datasource in our schema

diff --git a/src/packages/cli/src/__tests__/version.test.ts b/src/packages/cli/src/__tests__/version.test.ts
@@ -9,12 +9,16 @@ const ctx = Context.new().add(consoleContext()).assemble()
 const testIf = (condition: boolean) => (condition ? test : test.skip)
 const useNAPI = process.env.PRISMA_FORCE_NAPI === 'true'
 const version = '4165db0d1bddd480461f721ad5447bb261727728'
+
 describe('version', () => {
+
   // N-API Tests
+
   testIf(useNAPI)('basic version (N-API)', async () => {
     const data = await ctx.cli('--version')
     expect(cleanSnapshot(data.stdout)).toMatchSnapshot()
   })
+
   testIf(useNAPI)(
     'version with custom binaries (N-API)',
     async () => {
@@ -54,6 +58,7 @@ describe('version', () => {
   )
 
   // Binary Tests
+
   testIf(!useNAPI)('basic version', async () => {
     const data = await ctx.cli('--version')
     expect(cleanSnapshot(data.stdout)).toMatchSnapshot()

diff --git a/src/packages/client/src/__tests__/generation/generator.test.ts b/src/packages/client/src/__tests__/generation/generator.test.ts
@@ -180,6 +180,7 @@ describe('generator', () => {
       ).toMatchInlineSnapshot(`doesnotexist.prisma does not exist`)
     }
   })
+
   test('mongo', async () => {
     const prismaClientTarget = path.join(
       __dirname,

diff --git a/src/packages/client/src/__tests__/integration/errors/error-link/test.ts b/src/packages/client/src/__tests__/integration/errors/error-link/test.ts
@@ -1,7 +1,7 @@
 import { getTestClient } from '../../../../utils/getTestClient'
 
 test('error-link', async () => {
-  // TODO triggerPanic has not been implemented for N-API
+  // TODO triggerPanic has not been implemented for Node-API: https://github.com/prisma/prisma/issues/7810
   if (process.env.PRISMA_FORCE_NAPI === 'true') {
     return
   }

diff --git a/src/packages/client/src/__tests__/integration/errors/invalid-input/test.ts b/src/packages/client/src/__tests__/integration/errors/invalid-input/test.ts
@@ -16,6 +16,7 @@ test('invalid-input', async () => {
       },
     })
   } catch (e) {
+    // TODO The error output should be identical for both Node-API libary and binary: https://github.com/prisma/prisma/issues/7811
     if (process.env.PRISMA_FORCE_NAPI) {
       expect(e).toMatchInlineSnapshot(`
 

diff --git a/...rrors/missing-binary-native/schema.prisma → ...-binary-native-binaryTarget/schema.prisma b/...rrors/missing-binary-native/schema.prisma → ...-binary-native-binaryTarget/schema.prisma
diff --git a/...tion/errors/missing-binary-native/test.ts → ...issing-binary-native-binaryTarget/test.ts b/...tion/errors/missing-binary-native/test.ts → ...issing-binary-native-binaryTarget/test.ts
@@ -3,7 +3,7 @@ import fs from 'fs'
 import path from 'path'
 import { generateTestClient } from '../../../../utils/getTestClient'
 
-test('missing-binary-native', async () => {
+test('missing binary, native binaryTarget', async () => {
   expect.assertions(1)
   await generateTestClient()
 
@@ -30,6 +30,8 @@ test('missing-binary-native', async () => {
       },
     ],
   })
+
+
   if (process.env.PRISMA_FORCE_NAPI) {
     await expect(async () => {
       await prisma.user.findMany()
@@ -42,17 +44,17 @@ test('missing-binary-native', async () => {
                                     You incorrectly pinned it to TEST_PLATFORM
 
                                     This probably happens, because you built Prisma Client on a different platform.
-                                    (Prisma Client looked in "/client/src/__tests__/integration/errors/missing-binary-native/node_modules/@prisma/client/runtime/libquery_engine_napi-TEST_PLATFORM.so.node")
+                                    (Prisma Client looked in "/client/src/__tests__/integration/errors/missing-binary-native-binaryTarget/node_modules/@prisma/client/runtime/libquery_engine_napi-TEST_PLATFORM.so.node")
 
                                     Searched Locations:
 
-                                      /client/src/__tests__/integration/errors/missing-binary-native/node_modules/.prisma/client
-                                      /client/src/__tests__/integration/errors/missing-binary-native/node_modules/@prisma/client/runtime
-                                      /client/src/__tests__/integration/errors/missing-binary-native/node_modules/@prisma/client
-                                      /client/src/__tests__/integration/errors/missing-binary-native/node_modules/.prisma/client
-                                      /client/src/__tests__/integration/errors/missing-binary-native
+                                      /client/src/__tests__/integration/errors/missing-binary-native-binaryTarget/node_modules/.prisma/client
+                                      /client/src/__tests__/integration/errors/missing-binary-native-binaryTarget/node_modules/@prisma/client/runtime
+                                      /client/src/__tests__/integration/errors/missing-binary-native-binaryTarget/node_modules/@prisma/client
+                                      /client/src/__tests__/integration/errors/missing-binary-native-binaryTarget/node_modules/.prisma/client
+                                      /client/src/__tests__/integration/errors/missing-binary-native-binaryTarget
                                       /tmp/prisma-engines
-                                      /client/src/__tests__/integration/errors/missing-binary-native/node_modules/.prisma/client
+                                      /client/src/__tests__/integration/errors/missing-binary-native-binaryTarget/node_modules/.prisma/client
 
                                     You already added the platform "native" to the "generator" block
                                     in the "schema.prisma" file as described in https://pris.ly/d/client-generator,
@@ -70,17 +72,17 @@ test('missing-binary-native', async () => {
 
               Query engine binary for current platform "TEST_PLATFORM" could not be found.
             This probably happens, because you built Prisma Client on a different platform.
-            (Prisma Client looked in "/client/src/__tests__/integration/errors/missing-binary-native/node_modules/@prisma/client/runtime/query-engine-TEST_PLATFORM")
+            (Prisma Client looked in "/client/src/__tests__/integration/errors/missing-binary-native-binaryTarget/node_modules/@prisma/client/runtime/query-engine-TEST_PLATFORM")
 
             Searched Locations:
 
-              /client/src/__tests__/integration/errors/missing-binary-native/node_modules/.prisma/client
-              /client/src/__tests__/integration/errors/missing-binary-native/node_modules/@prisma/client/runtime
-              /client/src/__tests__/integration/errors/missing-binary-native/node_modules/@prisma/client
-              /client/src/__tests__/integration/errors/missing-binary-native/node_modules/.prisma/client
-              /client/src/__tests__/integration/errors/missing-binary-native
+              /client/src/__tests__/integration/errors/missing-binary-native-binaryTarget/node_modules/.prisma/client
+              /client/src/__tests__/integration/errors/missing-binary-native-binaryTarget/node_modules/@prisma/client/runtime
+              /client/src/__tests__/integration/errors/missing-binary-native-binaryTarget/node_modules/@prisma/client
+              /client/src/__tests__/integration/errors/missing-binary-native-binaryTarget/node_modules/.prisma/client
+              /client/src/__tests__/integration/errors/missing-binary-native-binaryTarget
               /tmp/prisma-engines
-              /client/src/__tests__/integration/errors/missing-binary-native/node_modules/.prisma/client
+              /client/src/__tests__/integration/errors/missing-binary-native-binaryTarget/node_modules/.prisma/client
 
             You already added the platform "native" to the "generator" block
             in the "schema.prisma" file as described in https://pris.ly/d/client-generator,

diff --git a/src/packages/client/src/__tests__/integration/errors/missing-binary/test.ts b/src/packages/client/src/__tests__/integration/errors/missing-binary/test.ts
@@ -30,6 +30,8 @@ test('missing-binary', async () => {
       },
     ],
   })
+  // TODO Error should not be as fundamentally different here as the test snapshots indicate
+  // TODO The error messages here are also not good (correct) and should be fixed
   if (process.env.PRISMA_FORCE_NAPI) {
     await expect(async () => {
       await prisma.user.findMany()

diff --git a/src/packages/client/src/__tests__/integration/errors/too-many-engines/test.ts b/src/packages/client/src/__tests__/integration/errors/too-many-engines/test.ts
@@ -19,6 +19,7 @@ test('raw-transaction: queryRaw', async () => {
     client.$disconnect()
   }
   if (process.env.PRISMA_FORCE_NAPI) {
+    // TODO Should this really not be implemented? https://github.com/prisma/prisma/issues/7814
     expect(warnings).toMatchInlineSnapshot(`Array []`)
   } else {
     expect(warnings).toMatchInlineSnapshot(`

diff --git a/src/packages/client/src/__tests__/integration/happy/chmod/test.ts b/src/packages/client/src/__tests__/integration/happy/chmod/test.ts
@@ -3,6 +3,7 @@ import fs from 'fs'
 import path from 'path'
 import { generateTestClient } from '../../../../utils/getTestClient'
 
+// Tests that no error is being thrown when the binary is manually set to chmod 644 because Client fixes that itself
 test('chmod', async () => {
   await generateTestClient()
   const platform = await getPlatform()
@@ -28,4 +29,6 @@ test('chmod', async () => {
   await prisma.user.findMany()
 
   await prisma.$disconnect()
+
+  // TODO expect that chmod is now not 644 any more
 })
diff --git a/src/packages/client/src/__tests__/integration/happy/restart/test.ts b/src/packages/client/src/__tests__/integration/happy/restart/test.ts
@@ -1,24 +1,31 @@
 import { getTestClient } from '../../../../utils/getTestClient'
 
+// Does Prisma Client restart the QE when it is killed for some reason?
 test('restart', async () => {
+  // No child process for Node-API, so nothing that can be killed or tested
+  if(process.env.PRISMA_FORCE_NAPI === 'true') {
+    return
+  }
+
   const PrismaClient = await getTestClient()
   const db = new PrismaClient({
     errorFormat: 'colorless',
   })
   await db.user.findMany()
-  if (!process.env.PRISMA_FORCE_NAPI) {
-    db._engine.child.kill()
-    await new Promise((r) => setTimeout(r, 1000))
-  }
+
+  // kill the binary child process
+  db._engine.child.kill()
+  await new Promise((r) => setTimeout(r, 1000))
+
   const result = await db.user.findMany()
   expect(result.length).toBeGreaterThan(0)
 
-  if (!process.env.PRISMA_FORCE_NAPI) {
-    for (let i = 0; i < 7; i++) {
-      db._engine.child.kill()
-      await new Promise((r) => setTimeout(r, 200))
-    }
+  // kill the binary child process again, to make sure it also comes back when engine is killed multiple times
+  for (let i = 0; i < 7; i++) {
+    db._engine.child.kill()
+    await new Promise((r) => setTimeout(r, 200))
   }
+
   const result2 = await db.user.findMany()
   expect(result2).toMatchInlineSnapshot(`
     Array [

diff --git a/...kages/client/src/__tests__/engine.test.ts → ...s/client/src/__tests__/nodeEngine.test.ts b/...kages/client/src/__tests__/engine.test.ts → ...s/client/src/__tests__/nodeEngine.test.ts
@@ -1,11 +1,15 @@
 import { NodeEngine } from '@prisma/engine-core/dist/NodeEngine'
 import path from 'path'
 
-describe('engine', () => {
+describe('NodeEngine', () => {
   test('should error correctly with invalid flags', async () => {
+
+    // Skip for Node-API library
+    // TODO Better scoping when to run this test so this conditional is not necessary
     if (process.env.PRISMA_FORCE_NAPI === 'true') {
       return
     }
+
     try {
       const engine = new NodeEngine({
         flags: ['--flag-that-does-not-exist'],

diff --git a/src/packages/client/src/generation/TSClient/TSClient.ts b/src/packages/client/src/generation/TSClient/TSClient.ts
@@ -66,6 +66,8 @@ export class TSClient implements Generatable {
       datasourceNames: this.options.datasources.map((d) => d.name),
       activeProvider: this.options.activeProvider,
     }
+
+    // Node-API env var
     if (
       process.env.PRISMA_FORCE_NAPI &&
       !config.generator?.previewFeatures.includes('nApi')

diff --git a/src/packages/sdk/src/engine-commands/getConfig.ts b/src/packages/sdk/src/engine-commands/getConfig.ts
@@ -39,7 +39,9 @@ export class GetConfigError extends Error {
 export async function getConfig(
   options: GetConfigOptions,
 ): Promise<ConfigMetaFormat> {
+
   const useNapi = process.env.PRISMA_FORCE_NAPI === 'true'
+
   let data: ConfigMetaFormat | undefined
   if (useNapi) {
     data = await getConfigNAPI(options)
@@ -48,6 +50,8 @@ export async function getConfig(
   }
 
   if (!data) throw new GetConfigError(`Failed to return any data`)
+
+  // TODO This has been outdated for ages and needs to be handled differently and/or removed
   if (
     data.datasources?.[0]?.provider?.[0] === 'sqlite' &&
     data.generators.some((g) => g.previewFeatures.includes('createMany'))