Skip to content

Conversation

@giancarloromeo
Copy link
Contributor

@giancarloromeo giancarloromeo commented Jun 26, 2025

What do these changes do?

This PR fixes hanging tests due to a missing initialization of AppServer mock in Celery tests.

image

The main issue here is that CI didn't detect the problem, since a cancelled test gave Unit Test passing.

Related issue/s

How to test

cd packages/celery-library
make tests

Dev-ops

@giancarloromeo giancarloromeo added this to the Engage milestone Jun 26, 2025
@giancarloromeo giancarloromeo self-assigned this Jun 26, 2025
@codecov
Copy link

codecov bot commented Jun 26, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.69%. Comparing base (6041cec) to head (a959a37).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7989      +/-   ##
==========================================
+ Coverage   87.89%   89.69%   +1.79%     
==========================================
  Files        1842     1246     -596     
  Lines       71060    53101   -17959     
  Branches     1231      199    -1032     
==========================================
- Hits        62460    47627   -14833     
+ Misses       8244     5407    -2837     
+ Partials      356       67     -289     
Flag Coverage Δ
integrationtests 64.34% <ø> (+0.04%) ⬆️
unittests 87.81% <ø> (+1.31%) ⬆️
Components Coverage Δ
api ∅ <ø> (∅)
pkg_aws_library ∅ <ø> (∅)
pkg_celery_library 81.08% <ø> (∅)
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 85.05% <ø> (-0.06%) ⬇️
agent 96.29% <ø> (ø)
api_server 92.64% <ø> (ø)
autoscaling 96.03% <ø> (ø)
catalog 92.29% <ø> (ø)
clusters_keeper 99.13% <ø> (ø)
dask_sidecar 92.35% <ø> (ø)
datcore_adapter 97.94% <ø> (ø)
director 76.73% <ø> (ø)
director_v2 91.12% <ø> (+0.01%) ⬆️
dynamic_scheduler 96.69% <ø> (ø)
dynamic_sidecar 90.09% <ø> (ø)
efs_guardian 89.65% <ø> (ø)
invitations 93.60% <ø> (ø)
payments 92.57% <ø> (ø)
resource_usage_tracker 89.00% <ø> (-0.17%) ⬇️
storage 86.52% <ø> (+0.28%) ⬆️
webclient ∅ <ø> (∅)
webserver 87.64% <ø> (-0.02%) ⬇️

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 6041cec...a959a37. 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.

@giancarloromeo giancarloromeo marked this pull request as ready for review June 26, 2025 13:15
@sonarqubecloud
Copy link

@giancarloromeo giancarloromeo enabled auto-merge (squash) June 26, 2025 13:16
Copy link
Contributor

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 addresses a hanging test issue by properly initializing the mock AppServer’s lifespan in the Celery test fixture.

  • Signals test startup completion via startup_completed_event.set()
  • Waits on shutdown_event to allow clean teardown and prevent hangs

Copy link
Collaborator

@matusdrobuliak66 matusdrobuliak66 left a comment

Choose a reason for hiding this comment

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

👍 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.

Thanks for the quick fix. Let's get this in as quickly as possible as this is blocking deployment to master if I am not mistaken

@giancarloromeo giancarloromeo changed the title 🐛 Fix Celery app server mock 🐛 Fix app server mock in celery-library Jun 26, 2025
@giancarloromeo giancarloromeo merged commit 383c511 into ITISFoundation:master Jun 26, 2025
96 of 97 checks passed
async def lifespan(self, startup_completed_event: threading.Event) -> None:
pass
startup_completed_event.set()
await self.shutdown_event.wait() # wait for shutdown
Copy link
Member

Choose a reason for hiding this comment

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

and is there indeed a timeout in the test in case this fails?

@giancarloromeo giancarloromeo deleted the fix-celery-app-server-mock branch June 26, 2025 13:46
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Aug 5, 2025
88 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants