-
Notifications
You must be signed in to change notification settings - Fork 44
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
Issue #339 #340
Issue #339 #340
Conversation
Status checks for running postgres depend on pg_ctl status code, not on pg_ctl log language
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also
- with current changes, please check if we can rid of the https://github.com/ClearcodeHQ/pytest-postgresql/blob/master/src/pytest_postgresql/executor.py#L147 usage all together.
- Either single test, or separate test env with system locales set to ie german.
src/pytest_postgresql/executor.py
Outdated
if start_info in content: | ||
break | ||
status_code = subprocess.getstatusoutput( | ||
'{pg_ctl} status -D {datadir}'.format( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use f-string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another question, can we simply rely on self.running method instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only self.running is used now
src/pytest_postgresql/executor.py
Outdated
@@ -176,10 +176,12 @@ def wait_for_postgres(self): | |||
start_info = 'database system is ready to accept connections' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
then this won't be needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, line 173, will we need logfile check here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
both deleted
The executor is now tested with two environment locales |
Status checks for running postgres depend on pg_ctl status code, not on pg_ctl log language
Fixes #339.
Changes proposed.