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

No client override during migrate:make #5109

Merged
merged 3 commits into from Apr 11, 2022

Conversation

OlivierCavadenti
Copy link
Collaborator

- During migrate:make client is set to 'sqlite3' only if client
is not setted in options and configuration.
@coveralls
Copy link

coveralls commented Apr 11, 2022

Coverage Status

Coverage increased (+0.01%) to 92.273% when pulling c0dcb78 on OlivierCavadenti:fix-client-override into 43dcc79 on knex:master.

@kibertoad
Copy link
Collaborator

Tests are failing.

@OlivierCavadenti
Copy link
Collaborator Author

(by the way we can see that coveralls comments works :D. I switch for detailed to short comments)

@OlivierCavadenti OlivierCavadenti merged commit 1794ea8 into knex:master Apr 11, 2022
@OlivierCavadenti OlivierCavadenti deleted the fix-client-override branch April 11, 2022 18:29
@sholladay
Copy link
Contributor

sholladay commented Apr 13, 2022

@kibertoad this PR and release 1.0.6 did not fix the problem.

At a glance, I believe the reason that the new test in this PR did not catch the problem is because, unlike many Knex users' projects, the Knex repo itself does have @vscode/sqlite3 installed. So you'll never get the "cannot find module" error, because Node can find it. And the test doesn't truly check which client is being used, even though it probably should.

Ideally, the testing infrastructure would scaffold a new temporary directory with its own package.json (etc) in /tmp for each test or at least each suite, to more easily reproduce a true user experience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Knex imports sqlite in migrate:make when told to use pg
4 participants