Skip to content

♻️ Maintenance: pytest-simcore initial cleanup #5986

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

Merged

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Jun 21, 2024

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

@pcrespov pcrespov force-pushed the mai/pytest-simcore-refactoring branch from d3a6cfc to 3c1f622 Compare June 21, 2024 14:01
@pcrespov pcrespov self-assigned this Jun 21, 2024
@pcrespov pcrespov changed the title WIP: Mai/pytest simcore refactoring WIP: ♻️ Maintenance: pytest-simcore cleanup Jun 21, 2024
@pcrespov pcrespov added this to the South Island Iced Tea milestone Jun 21, 2024
@pcrespov pcrespov added the t:maintenance Some planned maintenance work label Jun 21, 2024
@pcrespov pcrespov changed the title WIP: ♻️ Maintenance: pytest-simcore cleanup ♻️ Maintenance: pytest-simcore cleanup Jun 21, 2024
@pcrespov pcrespov changed the title ♻️ Maintenance: pytest-simcore cleanup ♻️ Maintenance: pytest-simcore initial cleanup Jun 21, 2024
@pcrespov pcrespov marked this pull request as ready for review June 21, 2024 16:03
@pcrespov pcrespov enabled auto-merge (squash) June 21, 2024 16:08
Copy link

codecov bot commented Jun 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

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

@@            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 Coverage Δ
integrationtests 64.7% <ø> (?)
unittests 85.8% <100.0%> (+1.3%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...ings-library/src/settings_library/comp_services.py 72.7% <100.0%> (ø)
...gs-library/src/settings_library/docker_registry.py 95.6% <100.0%> (ø)
...ges/settings-library/src/settings_library/email.py 90.0% <100.0%> (ø)
...ettings-library/src/settings_library/node_ports.py 86.2% <100.0%> (ø)

... and 1383 files with indirect coverage changes

Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor

@GitHK GitHK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@pcrespov
Copy link
Member Author

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
Copy link
Contributor

GitHK commented Jun 24, 2024

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
Copy link
Member Author

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 :-)

@pcrespov pcrespov disabled auto-merge June 24, 2024 08:33
@pcrespov pcrespov force-pushed the mai/pytest-simcore-refactoring branch from a34ff6e to 46b089a Compare June 24, 2024 19:28
@pcrespov pcrespov enabled auto-merge (squash) June 24, 2024 19:28
@pcrespov pcrespov requested a review from GitHK June 25, 2024 08:27
Copy link
Contributor

@GitHK GitHK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice, thanks!

Copy link
Contributor

@bisgaard-itis bisgaard-itis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, thanks a lot for the effort

@pcrespov pcrespov disabled auto-merge June 25, 2024 11:55
@pcrespov pcrespov merged commit ad25d00 into ITISFoundation:master Jun 25, 2024
56 checks passed
@pcrespov pcrespov deleted the mai/pytest-simcore-refactoring branch June 26, 2024 12:34
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Jul 5, 2024
26 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t:maintenance Some planned maintenance work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants