search

ITISFoundation / osparc-simcore

🐼 osparc-simcore simulation framework
https://osparc.io
MIT License
44 stars 26 forks source link

♻️ Maintenance: `pytest-simcore` initial cleanup #5986

Closed pcrespov closed 6 days ago

pcrespov commented 1 week ago

What do these changes do?

Following a discussion with @sanderegg on pytest-simcore, this PR performs an initial cleanup of the module according to our some proposed conventions. The mid-term goal is to simplify common fixtures and tests. However, since the changes are quite extensive, we are breaking them down into separate steps. This approach also allows everyone to stay informed and discuss the new conventions as we proceed.

  1. Listing of pytest_plugins must be sorted alphabetical
  2. Prefix pytest_simcore.helpers.utils_ is redundant. We will drop utils_.
    1. This PR changes some of the them (the most trivial ones)
    2. logging_utils and playwright_utils moved to pytest_simcore.helpers now
  3. pytest_addoption and its associated fixture must be together in the associated plugin module (e.g. --keep-docker-up and keep_docker_up)
  4. Retires (hides) packages/pytest-simcore/src/pytest_simcore/monkeypatch_extra.py: monkeypatch must have function context!

Related issue/s

How to test

All tests should run as before

Dev-ops checklist

codecov[bot] commented 1 week ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 87.8%. Comparing base (cafbf96) to head (fd30062). Report is 296 commits behind head on master.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/ITISFoundation/osparc-simcore/pull/5986/graphs/tree.svg?width=650&height=150&src=pr&token=h1rOE8q7ic&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ITISFoundation)](https://app.codecov.io/gh/ITISFoundation/osparc-simcore/pull/5986?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ITISFoundation) ```diff @@ Coverage Diff @@ ## master #5986 +/- ## ========================================= + Coverage 84.5% 87.8% +3.2% ========================================= Files 10 1415 +1405 Lines 214 58095 +57881 Branches 25 1340 +1315 ========================================= + Hits 181 51047 +50866 - Misses 23 6758 +6735 - Partials 10 290 +280 ``` | [Flag](https://app.codecov.io/gh/ITISFoundation/osparc-simcore/pull/5986/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ITISFoundation) | Coverage Δ | | |---|---|---| | [integrationtests](https://app.codecov.io/gh/ITISFoundation/osparc-simcore/pull/5986/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ITISFoundation) | `64.7% <ø> (?)` | | | [unittests](https://app.codecov.io/gh/ITISFoundation/osparc-simcore/pull/5986/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ITISFoundation) | `85.8% <100.0%> (+1.3%)` | :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=ITISFoundation#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/ITISFoundation/osparc-simcore/pull/5986?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ITISFoundation) | Coverage Δ | | |---|---|---| | [...ings-library/src/settings\_library/comp\_services.py](https://app.codecov.io/gh/ITISFoundation/osparc-simcore/pull/5986?src=pr&el=tree&filepath=packages%2Fsettings-library%2Fsrc%2Fsettings_library%2Fcomp_services.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ITISFoundation#diff-cGFja2FnZXMvc2V0dGluZ3MtbGlicmFyeS9zcmMvc2V0dGluZ3NfbGlicmFyeS9jb21wX3NlcnZpY2VzLnB5) | `72.7% <100.0%> (ø)` | | | [...gs-library/src/settings\_library/docker\_registry.py](https://app.codecov.io/gh/ITISFoundation/osparc-simcore/pull/5986?src=pr&el=tree&filepath=packages%2Fsettings-library%2Fsrc%2Fsettings_library%2Fdocker_registry.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ITISFoundation#diff-cGFja2FnZXMvc2V0dGluZ3MtbGlicmFyeS9zcmMvc2V0dGluZ3NfbGlicmFyeS9kb2NrZXJfcmVnaXN0cnkucHk=) | `95.6% <100.0%> (ø)` | | | [...ges/settings-library/src/settings\_library/email.py](https://app.codecov.io/gh/ITISFoundation/osparc-simcore/pull/5986?src=pr&el=tree&filepath=packages%2Fsettings-library%2Fsrc%2Fsettings_library%2Femail.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ITISFoundation#diff-cGFja2FnZXMvc2V0dGluZ3MtbGlicmFyeS9zcmMvc2V0dGluZ3NfbGlicmFyeS9lbWFpbC5weQ==) | `90.0% <100.0%> (ø)` | | | [...ettings-library/src/settings\_library/node\_ports.py](https://app.codecov.io/gh/ITISFoundation/osparc-simcore/pull/5986?src=pr&el=tree&filepath=packages%2Fsettings-library%2Fsrc%2Fsettings_library%2Fnode_ports.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ITISFoundation#diff-cGFja2FnZXMvc2V0dGluZ3MtbGlicmFyeS9zcmMvc2V0dGluZ3NfbGlicmFyeS9ub2RlX3BvcnRzLnB5) | `86.2% <100.0%> (ø)` | | ... and [1383 files with indirect coverage changes](https://app.codecov.io/gh/ITISFoundation/osparc-simcore/pull/5986/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ITISFoundation)
pcrespov commented 1 week ago

OK, I guess. If we change the convention why don't we have a PR which changes all of them?

@GitHK there were too many changes involved so I will be doing it in successive PRs

GitHK commented 1 week ago

OK, I guess. If we change the convention why don't we have a PR which changes all of them?

@GitHK there were too many changes involved so I will be doing it in successive PRs

For future changes, if someone makes a decision, it would be way better to apply it to all the involved pieces of code instead of having it spread out like this. It's easier for who was not involved in the decision process to follow it up and "emulate it", avoiding the introduction of the "old convention".

pcrespov commented 1 week ago

OK, I guess. If we change the convention why don't we have a PR which changes all of them?

@GitHK there were too many changes involved so I will be doing it in successive PRs

For future changes, if someone makes a decision, it would be way better to apply it to all the involved pieces of code instead of having it spread out like this. It's easier for who was not involved in the decision process to follow it up and "emulate it", avoiding the introduction of the "old convention".

@GitHK This PR is the "decision process". You have a saying if you agree or not on this. That is what I mention in the description. Yet another reason not to do all the changes at once :-)