Skip to content
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

Fix driver adapters tests after migration of driver adapters to prisma #4398

Merged
merged 5 commits into from
Oct 30, 2023

Conversation

miguelff
Copy link
Contributor

@miguelff miguelff commented Oct 28, 2023

compiles driver adapters against prisma/prisma#21642

@miguelff miguelff added this to the 5.6.0 milestone Oct 28, 2023
@miguelff miguelff requested a review from a team as a code owner October 28, 2023 10:01
@miguelff miguelff requested review from jkomyno and Druue and removed request for a team October 28, 2023 10:01
@codspeed-hq
Copy link

codspeed-hq bot commented Oct 28, 2023

CodSpeed Performance Report

Merging #4398 will not alter performance

Comparing fix-tests (32f778d) with main (79b5ee0)

Summary

✅ 11 untouched benchmarks


build-driver-adapters: ensure-prisma-present
@echo "Building driver adapters..."
@cd ../prisma && pnpm --filter "*adapter*" i && pnpm --filter "*adapter*" build
@cd ../prisma && pnpm --filter "*adapter*" i
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@millsp I observed that if I don't install the deps from prisma, I get an error if running pnpm build from prisma-engines/query-engine/driver-adapters. Why can this be, given that all the projects that I'm trying to build are part of the workspace?

Copy link
Member

Choose a reason for hiding this comment

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

I guess it is because they are still two different workspaces (since they have two different workspace files), so pnpm will not recursively install once it exits on the other workspace.

"scripts": {
"build": "pnpm -r run build",
"lint": "pnpm -r run lint",
"clean": "git clean -nXd -e !query-engine/driver-adapters"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this clean command still relevant?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is, I expect to use it from time to time, yes.

@janpio
Copy link
Contributor

janpio commented Oct 30, 2023

Is WASM build / prisma-schema-wasm build for commit (pull_request) failing because of this PR? Doesn't seem to be broken on main I think.

@janpio
Copy link
Contributor

janpio commented Oct 30, 2023

Similar question for Schema Engine tests, Vitess 8 and MariaDB on Windows. Are these just flakey, or did the PR somehoe break them?

@miguelff
Copy link
Contributor Author

Similar question for Schema Engine tests, Vitess 8 and MariaDB on Windows. Are these just flakey, or did the PR somehoe break them?

Seem flakey

Is WASM build / prisma-schema-wasm build for commit (pull_request) failing because of this PR? Doesn't seem to be broken on main I think.

Still unknown, failed consistenly, I attempt a retry just before you commented. I'm checking.

Comment on lines +2 to +7
- '../../../prisma/packages/adapter-libsql'
- '../../../prisma/packages/adapter-neon'
- '../../../prisma/packages/adapter-pg'
- '../../../prisma/packages/adapter-planetscale'
- '../../../prisma/packages/driver-adapter-utils'
- '../../../prisma/packages/debug'
Copy link
Member

Choose a reason for hiding this comment

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

So you don't need to add it in the future

Suggested change
- '../../../prisma/packages/adapter-libsql'
- '../../../prisma/packages/adapter-neon'
- '../../../prisma/packages/adapter-pg'
- '../../../prisma/packages/adapter-planetscale'
- '../../../prisma/packages/driver-adapter-utils'
- '../../../prisma/packages/debug'
- '../../../prisma/packages/*'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wouldn't this require to build all the packages in the prisma monorepo each time I rebuild driver adapters?

Copy link
Member

Choose a reason for hiding this comment

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

I would assume that pnpm would only install the deps that you are actually using, the best is to give it a try. Looks good either way.

@miguelff
Copy link
Contributor Author

Is WASM build / prisma-schema-wasm build for commit (pull_request)

Fixed.

@miguelff miguelff self-assigned this Oct 30, 2023
otherwise @esbuild-register is not found despite being defined as a devDependency within driver adapters @millsp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants