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

WIP: ♻️ mai/refactor products #7263

Draft
wants to merge 28 commits into
base: master
Choose a base branch
from

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Feb 21, 2025

What do these changes do?

This PR refactors simcore_service_webserver.products domain and extends its test coverage over 90%

♻️ refactors simcore_service_webserver.products package skeleton to fit

  • controller layer:
    • _rest
      • moved _invitation_rest -> invitations domain
    • _rpc
  • repository layer: _repository
    • upgraded to asyncpg
  • service layer: _service, products_service (public)
  • domain models: _models, models (public)
    • upgraded fields to Annotated
  • http server modules: web
    • _web_events: web.Application slots
    • _web_middlewares: web.Application middlewares
    • _web_helpers: helpers that wraps service layer on an aiohttp.web inteface.
    • products_web (public): access to some of the helpers
  • domain bootstrap/setup: plugin
    • internal imports to reduce boot-time
  • extends testing to reach >90% coverage on products code

This is how the public inteface of this domain is suppose to be used

from ..products import products_web, products_service
from ..products.models import Product

async def some_handler(request: web.Request) 
      product: Product = products_web.get_current_product(request)
      products_web.get_product_template_path(request, template_name )
      # etc
     

async def other_func(app: web.Application):
     for product in products_service.list_products(app)
              # do something

🔨 enhances pytest_simcore.pydantic_models helpers

Enhances pytest_simcore.pydantic_models helpers to parametrize tests of examples from a package, module or class level using iter_model_examples_in_* iterators. assert_validation_model does all the standard test on the examples as well.

       @pytest.mark.parametrize(
            "model_cls, example_name, example_data",
            iter_model_examples_in_package(some_library), # <---
        )
        def test_model_examples_1(
            model_cls: type[BaseModel], example_name: str, example_data: Any
        ):
            assert_validation_model(
                model_cls, example_name=example_name, example_data=example_data
            )
            
       @pytest.mark.parametrize(
            "model_cls, example_name, example_data",
            iter_model_examples_in_module(some_library.models), # <---
        )
        def test_model_examples_2(
            model_cls: type[BaseModel], example_name: str, example_data: Any
        ):
            assert_validation_model(
                model_cls, example_name=example_name, example_data=example_data
            )

       @pytest.mark.parametrize(
            "model_cls, example_name, example_data",
            iter_model_examples_in_class(some_library.models.Book), # <---
        )
        def test_model_examples_3(
            model_cls: type[BaseModel], example_name: str, example_data: Any
        ):
            assert_validation_model(
                model_cls, example_name=example_name, example_data=example_data
            )
            

Related issue/s

How to test

Driving test

cd services/web/server
make install-dev

pytest -vv tests --cov=simcore_service_webserver --cov-config=/home/crespo/repos/osparc-simcore/.coveragerc  tests/unit/isolated/**/test_*product*.py

Dev-ops

None

@pcrespov pcrespov self-assigned this Feb 21, 2025
Copy link

codecov bot commented Feb 21, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.93%. Comparing base (731dd9a) to head (db2c77c).

❗ There is a different number of reports uploaded between BASE (731dd9a) and HEAD (db2c77c). Click for more details.

HEAD has 27 uploads less than BASE
Flag BASE (731dd9a) HEAD (db2c77c)
unittests 32 5
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #7263       +/-   ##
===========================================
- Coverage   87.17%   69.93%   -17.24%     
===========================================
  Files        1680      757      -923     
  Lines       65053    34301    -30752     
  Branches     1106      180      -926     
===========================================
- Hits        56709    23989    -32720     
- Misses       8030    10252     +2222     
+ Partials      314       60      -254     
Flag Coverage Δ *Carryforward flag
integrationtests 65.40% <ø> (-0.05%) ⬇️ Carriedforward from 731dd9a
unittests 93.00% <ø> (+6.82%) ⬆️

*This pull request uses carry forward flags. Click here to find out more.

Components Coverage Δ
api 76.84% <ø> (ø)
pkg_aws_library ∅ <ø> (∅)
pkg_dask_task_models_library 97.09% <ø> (ø)
pkg_models_library ∅ <ø> (∅)
pkg_notifications_library ∅ <ø> (∅)
pkg_postgres_database ∅ <ø> (∅)
pkg_service_integration ∅ <ø> (∅)
pkg_service_library ∅ <ø> (∅)
pkg_settings_library ∅ <ø> (∅)
pkg_simcore_sdk 77.30% <ø> (-8.16%) ⬇️
agent 96.46% <ø> (ø)
api_server ∅ <ø> (∅)
autoscaling ∅ <ø> (∅)
catalog 91.73% <ø> (ø)
clusters_keeper ∅ <ø> (∅)
dask_sidecar ∅ <ø> (∅)
datcore_adapter 98.06% <ø> (ø)
director ∅ <ø> (∅)
director_v2 78.50% <ø> (-12.76%) ⬇️
dynamic_scheduler ∅ <ø> (∅)
dynamic_sidecar 89.74% <ø> (ø)
efs_guardian ∅ <ø> (∅)
invitations ∅ <ø> (∅)
osparc_gateway_server ∅ <ø> (∅)
payments ∅ <ø> (∅)
resource_usage_tracker ∅ <ø> (∅)
storage ∅ <ø> (∅)
webclient ∅ <ø> (∅)
webserver 59.31% <ø> (-25.85%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 731dd9a...db2c77c. Read the comment docs.

@pcrespov pcrespov force-pushed the mai/refactor-products branch 2 times, most recently from 694fdaa to 4976951 Compare February 24, 2025 10:19
@pcrespov pcrespov added this to the The Awakening milestone Feb 24, 2025
@pcrespov pcrespov added the t:maintenance Some planned maintenance work label Feb 24, 2025
@pcrespov pcrespov force-pushed the mai/refactor-products branch 2 times, most recently from 64b79d8 to 07d794e Compare February 25, 2025 10:11
@pcrespov pcrespov force-pushed the mai/refactor-products branch from 07d794e to db2c77c Compare February 25, 2025 15:02
Copy link

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.

1 participant