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

working example #78

Open
wants to merge 33 commits into
base: master
Choose a base branch
from
Open

Conversation

Veritogen
Copy link

@Veritogen Veritogen commented Dec 18, 2023

Description

Fixes #77

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes (not just the CI)
  • Any dependent changes have been merged and published in downstream modules
  • I have provided a link to the issue this PR adresses in the Description section above (If there is none yet,
    create one !)
  • I have updated the changelog
  • I have labeled my PR using appropriate tags (in particular using status labels like Status: Code Review Needed, Business: Test Needed or Status: In Progress if you are still working on the PR)

@js-dieu I'd be grateful for your input (also on the questions I posed in the issue mentioned above).

@Veritogen Veritogen marked this pull request as draft December 18, 2023 14:16
Hannes Engelhardt and others added 13 commits December 20, 2023 10:54
properly close the connections on drop/garbage collection
properly close the connections on drop/garbage collection
…es not worth own commit)

Merge branch 'postgres-DB-handler' of github.com:einhundert/pytest-monitor into postgres-DB-handler
with-context: Remove the with-contexts inside the PostgresDBHandler and
do manual commit after cursor execution instead to prevent closing of
the database connection.

db-close: Add close() function to both db-handlers and the
PytestMonitorSession class in order to properly close all database
connections at the end of the tests. This happens in pytest-monitor.py
with the pytest_sessionfinish hook implementation that calls the
close() function on the PytestMonitorSession object.

env-id: The query for the env-id PostgresDBHandler.get_env_id() queries
for the env id in the database and gets a tuple from the query but does
return it without unwrapping the value inside. This leads to problems
as the value is meant to be unwrapped, therefore return the unwrapped
value.
This is a helper function for generating a session description in
session.py.
The session is tested for proper closing of the database connection.
The close() function is called from the hookimplementation in
pytest_sessionfinish().

The dbhandlers are tested for proper setup of the tables and in
consequence a working connection. The PostgresDBHandler requires a
running postgres server, that's what the docker-compose.yml file is for
@Veritogen Veritogen marked this pull request as ready for review June 13, 2024 09:21
@Veritogen
Copy link
Author

@js-dieu due to the effort of my colleague @lhpt2 we now have the feature fully tested and would like to merge changes that introduce a postgres db handler to pytest-monitor. Are there any steps we didn't to for a review? Unfortunately, I can't set the labels as expected in the PR checklist.

Merge with upstream master and update changelog.rst
@Veritogen
Copy link
Author

@js-dieu we also didn't set a release in the changelog.rst and just added "to be discussed". Will you set the release number or should we add something there?

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
2 Security Hotspots

See analysis details on SonarCloud

@lhpt2
Copy link

lhpt2 commented Jun 18, 2024

Notice: The quality gate failed because of literal passwords for the testing environment (which should not be a security issue as its only for testing purposes)

The memory profiler only reports Exceptions when it should report all
Exceptions that inherit from BaseExceptions.

This is ultimately reworked in a PR incorporating a simplified
memory profiler module into the codebase that fixes not only this issue
but also gives the possibility of getting measurements for failing
tests.

This workaround uses return values to work around the issue. This way
pytest-monitor can check for BaseExceptions and act accordingly.
Like described earlier the ultimate goal should probably be to replace
the whole memory profiler as proposed in this PR:
CFMTech#82
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.

Add postgres DB handler
3 participants