Skip to content

Conversation

@pcrespov
Copy link
Member

@pcrespov pcrespov commented Jan 9, 2025

What do these changes do?

TL;DR

Remove legacy app_config fixtures from web/server/tests

♻️ Removing app_config fixtures

This PR updates the web-server tests by removing outdated app_config fixtures. These fixtures were remnants from a time before the introduction of ApplicationSettings, when the application was initialized using configuration files. Although the functionality to initialize the app with config files was removed long ago, the test setup still relied on these legacy fixtures.

To bridge the gap during the transition, we used a converter function, monkeypatch_setenv_from_app_config (implemented as a fixture factory), which took the old configuration files and translated them into equivalent environment variables compatible with ApplicationSettings. This PR adapts the remaining fixtures depending mechanism so we can finally remove all app_config legacy from the code (and move on).

Remember that the workflow to create a client that tests the app in web/server/tests is

  1. app_environment fixture mocks test env vars
  2. web_server fixture creates app and runs on top of a TestServer
  3. client fixture produces a TestClient that connects to web_server

Other refactoring ♻️:

  • moving the sub-module projects._common.models
  • sorting and using new Annotated-fields in ApplicationSettings
  • annotated test arguments

Related issue/s

How to test

cd services/web/server
make install-dev
make test-dev-unit
make build
make test-dev-integration

Dev-ops

  • sorting .env-devel alphabetically

@pcrespov pcrespov self-assigned this Jan 9, 2025
@pcrespov pcrespov added the t:maintenance Some planned maintenance work label Jan 9, 2025
@pcrespov pcrespov added this to the Event Horizon milestone Jan 9, 2025
@pcrespov pcrespov changed the title WIP: ♻️ Mai/rm app config web tests WIP: ♻️ Maintenance: removes legacy app-config fixture tests from web-server Jan 9, 2025
@codecov
Copy link

codecov bot commented Jan 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.09%. Comparing base (8bda78f) to head (f6f3ed5).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7022      +/-   ##
==========================================
- Coverage   86.35%   83.09%   -3.26%     
==========================================
  Files        1226      669     -557     
  Lines       52507    32388   -20119     
  Branches     1271      262    -1009     
==========================================
- Hits        45344    26914   -18430     
+ Misses       6925     5414    -1511     
+ Partials      238       60     -178     
Flag Coverage Δ
integrationtests 64.85% <100.00%> (-0.01%) ⬇️
unittests 84.82% <100.00%> (-0.28%) ⬇️
Components Coverage Δ
api 76.84% <ø> (∅)
pkg_aws_library ∅ <ø> (∅)
pkg_dask_task_models_library ∅ <ø> (∅)
pkg_models_library ∅ <ø> (∅)
pkg_notifications_library ∅ <ø> (∅)
pkg_postgres_database ∅ <ø> (∅)
pkg_service_integration ∅ <ø> (∅)
pkg_service_library ∅ <ø> (∅)
pkg_settings_library ∅ <ø> (∅)
pkg_simcore_sdk 77.37% <ø> (-8.02%) ⬇️
agent ∅ <ø> (∅)
api_server ∅ <ø> (∅)
autoscaling ∅ <ø> (∅)
catalog ∅ <ø> (∅)
clusters_keeper ∅ <ø> (∅)
dask_sidecar ∅ <ø> (∅)
datcore_adapter ∅ <ø> (∅)
director ∅ <ø> (∅)
director_v2 78.77% <ø> (-12.61%) ⬇️
dynamic_scheduler ∅ <ø> (∅)
dynamic_sidecar 89.74% <ø> (ø)
efs_guardian ∅ <ø> (∅)
invitations ∅ <ø> (∅)
osparc_gateway_server ∅ <ø> (∅)
payments ∅ <ø> (∅)
resource_usage_tracker ∅ <ø> (∅)
storage ∅ <ø> (∅)
webclient ∅ <ø> (∅)
webserver 84.07% <100.00%> (+<0.01%) ⬆️

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 8bda78f...f6f3ed5. Read the comment docs.

@pcrespov pcrespov changed the title WIP: ♻️ Maintenance: removes legacy app-config fixture tests from web-server ♻️ Maintenance: removes legacy app-config fixture tests from web-server Jan 10, 2025
@pcrespov pcrespov marked this pull request as ready for review January 10, 2025 08:33
@pcrespov pcrespov enabled auto-merge (squash) January 10, 2025 08:33
@pcrespov pcrespov added the a:webserver webserver's codebase. Assigning the area is particularly useful for bugs label Jan 10, 2025
@odeimaiz odeimaiz disabled auto-merge January 10, 2025 09:13
Copy link
Member

@mrnicegyu11 mrnicegyu11 left a comment

Choose a reason for hiding this comment

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

lgtm

@pcrespov pcrespov force-pushed the mai/rm-app_config-web-tests branch from e47f418 to e1712c1 Compare January 10, 2025 10:42
@pcrespov pcrespov enabled auto-merge (squash) January 10, 2025 10:52
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.

Thanks a lot. Just some minor things and questions

@sonarqubecloud
Copy link

@pcrespov pcrespov merged commit 85c3281 into ITISFoundation:master Jan 10, 2025
89 of 93 checks passed
@pcrespov pcrespov deleted the mai/rm-app_config-web-tests branch January 10, 2025 19:16
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Jan 15, 2025
58 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a:webserver webserver's codebase. Assigning the area is particularly useful for bugs t:maintenance Some planned maintenance work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants