Skip to content

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Sep 12, 2025

What do these changes do?

This pull request is a spin-off from PR #8141, which focuses on refactoring the project.workbench and therefore includes many many changes. This PR can be merged independently and will help reduce the size of the original PR and therefore make it easier to review. The changes here are mostly enhancements and utilities and do not introduce critical modifications to existing features.

Type Hinting and Error Handling Improvements:

  • Updated type hinting in packages/simcore-sdk/src/simcore_sdk/node_ports_v2/port.py to use modern union syntax (e.g., FileLink | DownloadLink instead of tuples) and refactored error messages for clarity. This affects value validation and async value retrieval logic. [1] [2] [3]

Test Assertion Refactoring:

  • Replaced direct equality assertions in project-related tests with DeepDiff comparisons, allowing for exclusion of fields like lastChangeDate and ignoring order where necessary. This change enhances robustness and flexibility of test checks. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]

Database Table and Import Cleanups:

  • Updated services/director-v2/src/simcore_service_director_v2/modules/db/tables.py to use a tuple for __all__ and added missing imports for projects_nodes.

Test Fixture and Assertion Enhancements:

  • Added DeepDiff imports to relevant test files and replaced some equality assertions with custom helpers like assert_equal_ignoring_none for more accurate test validation. [1] [2] [3] [4]

Miscellaneous Fixes:

  • Improved error reporting for wallet credit errors by including additional context such as user_id, product_name, and project_id in exceptions.
  • Added missing labels and dataset fields to node output assertions and fixed test parameterization decorators. [1] [2]

Let me know if you'd like a deeper explanation of any specific change!

Related issue/s

How to test

Dev-ops

@pcrespov pcrespov added this to the Cheops milestone Sep 12, 2025
@pcrespov pcrespov self-assigned this Sep 12, 2025
Copy link

codecov bot commented Sep 12, 2025

Codecov Report

❌ Patch coverage is 66.66667% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.79%. Comparing base (e0f1ec9) to head (85d00cf).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8368      +/-   ##
==========================================
+ Coverage   87.87%   89.79%   +1.92%     
==========================================
  Files        1945     1320     -625     
  Lines       75626    55810   -19816     
  Branches     1321      175    -1146     
==========================================
- Hits        66454    50117   -16337     
+ Misses       8774     5635    -3139     
+ Partials      398       58     -340     
Flag Coverage Δ
integrationtests 63.93% <41.66%> (-0.01%) ⬇️
unittests 87.99% <58.33%> (+1.44%) ⬆️
Components Coverage Δ
pkg_aws_library ∅ <ø> (∅)
pkg_celery_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 84.99% <55.55%> (-0.05%) ⬇️
agent 93.53% <ø> (ø)
api_server 91.94% <ø> (ø)
autoscaling 95.77% <ø> (ø)
catalog 92.36% <ø> (ø)
clusters_keeper 99.13% <ø> (ø)
dask_sidecar 92.37% <ø> (+0.22%) ⬆️
datcore_adapter 97.94% <ø> (ø)
director 75.81% <ø> (-0.09%) ⬇️
director_v2 90.82% <100.00%> (-0.11%) ⬇️
dynamic_scheduler 96.27% <ø> (ø)
dynamic_sidecar 90.46% <ø> (ø)
efs_guardian 89.62% <ø> (ø)
invitations 91.44% <ø> (ø)
payments 92.61% <ø> (ø)
resource_usage_tracker 92.18% <ø> (ø)
storage 86.53% <ø> (-0.30%) ⬇️
webclient ∅ <ø> (∅)
webserver 87.99% <ø> (+0.04%) ⬆️

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 e0f1ec9...85d00cf. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

mergify bot commented Sep 12, 2025

🧪 CI Insights

Here's what we observed from your CI run for 85d00cf.

🟢 All jobs passed!

But CI Insights is watching 👀

@pcrespov pcrespov changed the title WIP: Is8141/workbench pr spinoff 2 ♻️ [Maintenance] Refactor Tests Utilities and Typing (Spin-off 2 from PR #8141) Sep 15, 2025
@pcrespov pcrespov added a:webserver webserver's codebase. Assigning the area is particularly useful for bugs t:maintenance Some planned maintenance work labels Sep 15, 2025
@pcrespov pcrespov marked this pull request as ready for review September 15, 2025 09:04
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR is a maintenance refactor focused on improving test utilities and type hinting across the codebase. It includes enhancements to type annotations, test assertion improvements, and minor code quality fixes as a spin-off from a larger PR.

  • Updates type hinting to use modern union syntax and improves error handling clarity
  • Replaces direct equality assertions with DeepDiff comparisons for more flexible test validation
  • Adds missing imports and fixes minor code style issues

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
services/web/server/tests/unit/with_dbs/04/workspaces/test_workspaces__moving_projects_between_workspaces.py Updates dictionary access to use .get() method for safer key access
services/web/server/tests/unit/with_dbs/04/workspaces/test_workspaces__folders_and_projects_crud.py Updates dictionary access to use .get() method for safer key access
services/web/server/tests/unit/with_dbs/04/notifications/test_notifications__db_comp_tasks_listening_task.py Refactors mock patching to use explicit import and patch.object()
services/web/server/tests/unit/with_dbs/02/test_projects_states_handlers.py Replaces equality assertions with DeepDiff comparisons and adds missing imports
services/web/server/tests/unit/with_dbs/02/test_projects_ports_handlers.py Replaces equality assertions with DeepDiff comparisons and fixes test decorator
services/web/server/tests/unit/with_dbs/02/test_projects_nodes_handlers__patch.py Adds missing fields to node output assertions and replaces equality with DeepDiff
services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers.py Replaces equality assertions with DeepDiff comparisons and adds utility imports
services/web/server/tests/data/fake-project.json Updates progress value from integer to float
services/director-v2/src/simcore_service_director_v2/utils/dags.py Adds assertion for workbench node validation
services/director-v2/src/simcore_service_director_v2/modules/db/tables.py Updates __all__ to tuple format and adds missing imports
services/director-v2/src/simcore_service_director_v2/modules/db/repositories/comp_tasks/_utils.py Enhances error context with additional fields
packages/simcore-sdk/src/simcore_sdk/node_ports_v2/port.py Updates type annotations to use modern union syntax and improves error messages

Copy link
Contributor

@giancarloromeo giancarloromeo 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
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.

thanks!

@pcrespov pcrespov added the 🤖-automerge marks PR as ready to be merged for Mergify label Sep 15, 2025
@pcrespov pcrespov enabled auto-merge (squash) September 15, 2025 17:31
@pcrespov
Copy link
Member Author

@mergify queue

Copy link
Contributor

mergify bot commented Sep 15, 2025

queue

🟠 Waiting for conditions to match

  • -closed [📌 queue requirement]
  • -conflict [📌 queue requirement]
  • -draft [📌 queue requirement]
  • any of: [📌 queue -> configuration change requirements]
    • -mergify-configuration-changed
    • check-success = Configuration changed
  • any of: [🔀 queue conditions]
    • all of: [📌 queue conditions of queue default]
      • #approved-reviews-by >= 2 [🛡 GitHub branch protection]
      • #approved-reviews-by>=2
      • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
      • #changes-requested-reviews-by=0
      • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
      • #review-threads-unresolved=0
      • -conflict
      • -draft
      • base=master
      • branch-protection-review-decision = APPROVED [🛡 GitHub branch protection]
      • label!=🤖-do-not-merge
      • label=🤖-automerge
      • any of: [🛡 GitHub branch protection]
        • check-skipped = deploy to dockerhub
        • check-neutral = deploy to dockerhub
        • check-success = deploy to dockerhub
      • any of: [🛡 GitHub branch protection]
        • check-success = system-tests
        • check-neutral = system-tests
        • check-skipped = system-tests
      • any of: [🛡 GitHub branch protection]
        • check-success = unit-tests
        • check-neutral = unit-tests
        • check-skipped = unit-tests
      • any of: [🛡 GitHub branch protection]
        • check-success = check OAS' are up to date
        • check-neutral = check OAS' are up to date
        • check-skipped = check OAS' are up to date
      • any of: [🛡 GitHub branch protection]
        • check-success = integration-tests
        • check-neutral = integration-tests
        • check-skipped = integration-tests
      • any of: [🛡 GitHub branch protection]
        • check-success = build-test-images (frontend) / build-test-images
        • check-neutral = build-test-images (frontend) / build-test-images
        • check-skipped = build-test-images (frontend) / build-test-images
      • any of: [🛡 GitHub branch protection]
        • check-success = SonarCloud Code Analysis
        • check-neutral = SonarCloud Code Analysis
        • check-skipped = SonarCloud Code Analysis

Copy link

@pcrespov pcrespov merged commit 7e42fd0 into ITISFoundation:master Sep 15, 2025
93 of 95 checks passed
@pcrespov pcrespov deleted the is8141/workbench-pr-spinoff-2 branch September 16, 2025 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖-automerge marks PR as ready to be merged for Mergify 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.

3 participants