GispoCoding / pytest-qgis

A pytest plugin for testing QGIS python plugins
GNU General Public License v2.0
29 stars 8 forks source link

Added clean_qgis_layer decorator back #64

Closed Joonalai closed 2 months ago

Joonalai commented 3 months ago

This PR:

Joonalai commented 2 months ago

@LKajan, @ismogis could you review this?

LKajan commented 2 months ago

I think this kind of approach would indeed be cleaner than figuring by the function name the need for layer cleaning.

This particular implementation seems to fail in cases where the layer fixture is using yield:

@pytest.fixture()
@clean_qgis_layer
def layer():
  layer = QgsLayer(...)
  yield layer
  print(f"teardown layer {layer}")

Could you find some alternative way for the implementation?

Have you studied what actually is causing these seg faults? Is it possible to fix the root cause instead?

codecov-commenter commented 2 months ago

Codecov Report

Attention: Patch coverage is 66.66667% with 3 lines in your changes missing coverage. Please review.

Project coverage is 57.53%. Comparing base (d782092) to head (a466e4c).

Files Patch % Lines
src/pytest_qgis/utils.py 66.66% 3 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #64 +/- ## ========================================== + Coverage 57.25% 57.53% +0.28% ========================================== Files 7 7 Lines 496 504 +8 ========================================== + Hits 284 290 +6 - Misses 212 214 +2 ``` | [Flag](https://app.codecov.io/gh/GispoCoding/pytest-qgis/pull/64/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GispoCoding) | Coverage Δ | | |---|---|---| | [pytest](https://app.codecov.io/gh/GispoCoding/pytest-qgis/pull/64/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GispoCoding) | `57.53% <66.66%> (+0.28%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=GispoCoding#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

LKajan commented 2 months ago

I added a mention that this doesn't work with fixtures that yield for now and made typing more precise. With these changes I think it is ok to merge until we find a better solution to make sure map layers don't cause seg faults.