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

use provided dname if available for _noproc #660

Closed
wants to merge 3 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/pytest_postgresql/executor_noop.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def version(self) -> Any:
# could be called before self.dbname will be created.
# Use default postgres database
with psycopg.connect(
dbname="postgres",
dbname=self.dbname or "postgres",
Copy link
Collaborator

Choose a reason for hiding this comment

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

but dbname will never be empty 🤔 And we'll attempt to create the database in question.... most probably prior to it's creation 🤔

Copy link
Author

Choose a reason for hiding this comment

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

Hmmm. Indeed, _noproc tries to create a database, and fails. I'd like to be able to use it with a basic database account, but the problem goes deeper than just connecting to the postgres database :-/

Copy link
Author

Choose a reason for hiding this comment

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

With test file test.py:

import pytest
from pytest_postgresql import factories

def test_hello(postgresql_noproc):
    assert True

Run test against an existing database:

# create a basic user
createuser -W pytest  # enter password: pytest
# with a database
createdb -O pytest pytest
# run test
pytest --postgresql-user=pytest --postgresql-password=pytest --postgresql-dbname=pytest --postgresql-use-database test.py

Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be dependant on the new flag you've added. rather than the self.dbname being null which it will not 🤔

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure. This means that the option has somehow two effects, but why not.

I've updated the patch.

user=self.user,
host=self.host,
port=self.port,
Expand Down