postgresql_noproc factory changes dbname to dbname + '_tmpl'

[oev81]

Hi!

Is this expected?

The yielded NoopExecutor instance has initial dbname and it's confusing.

[fizyk]

Ho @oev81 it is expected. Same as for the postgres_proc factory:

pytest-postgresql/src/pytest_postgresql/factories/process.py

Line 149 in 4e19c3e

dbname=template_dbname,

Although It might be better encapsulated though 🤔 somehow...
*proc factories should create a template database and fill it with initial data if given. And client fixture will create its own database out of the template before each test and destroy it afterwards.

https://github.com/ClearcodeHQ/pytest-postgresql#using-a-common-database-initialisation-between-tests

[oev81]

@fizyk
My usecase is to create a new database, whatever named, and then use its name in a separate connection for tests. And this is confusing, for a user, that the real dbname differs from the specified one. The approach of generating a dbname using a template is ok. But, there are few things to make it more clear:

• the dbname parameter of postgresql_no proc is not a real dbname. It plays role of a dbname prefix or something. So renaming to dbname_prefix or something seems reasonable.
• And it looks like that NoopExcecutor's dbname attribute should contain the real dbname.

[fizyk]

@oev81 do I understand correctly, that you're maintaining your test database outside the test code? If that's the case, then the only use of pytest-postgresql fixtures would be to maintain connections. In this case, using raw DatabaseJanitor would be more suitable, like pypi's warehouse is using 🤔

The thing is, that the fixtures main goal is to maintain the databases and the only difference between proc and noproc is that the latter does not manage the database server/process but rely on the developer to provide an instance with running database. And still attempt to create temp database /database pair.

And in fact it shouldn't matter that you'll pass the name for the future database to your code unless it'll start before your testing code (and fixtures) 🤔

[fizyk]

There's a related issue #653 and a PR #660

[oev81]

@fizyk Yes, DatabaseJanitor is more suitable. But this class looks like internal, and I thought Its better not to use it. Why? It has the version parameter. Although, it is not used anywhere (Maybe I didn't find a place), the parameter is mandatory and needs to be specified. On the other hand, the NoopExcecutor class contains logic for the automatic version detection, and if the version is really needed, using the factories.postgresql_noproc lets to not think about the version.

[fizyk]

@oev81 historically it was used 255681b#diff-47a715ffd93e4dc187da7b1a66fb6f41b7dc05427479c92d61409665dec52dc0 There was the small difference I had to account for between 9.2 and earlier postgresql versions supported at the time. Might be worth to make it optional again, though if that'd be needed again...