Skip to content

add deterministic methods and increase test coverage #35

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Nov 18, 2025
Merged

add deterministic methods and increase test coverage #35

merged 5 commits into from
Nov 18, 2025

Conversation

@filintod
Copy link

@filintod filintod commented Nov 14, 2025 •
edited
Loading

Also from asyncio big PR, adding deterministic methods that are shared between sync/async context. Also increase test coverage of some previously not tested paths

current coverage:

============================================================================================================================== tests coverage ===============================================================================================================================
_____________________________________________________________________________________________________________ coverage: platform darwin, python 3.13.2-final-0 ______________________________________________________________________________________________________________

Name                                                    Stmts   Miss Branch BrPart  Cover   Missing
---------------------------------------------------------------------------------------------------
durabletask/__init__.py                                     1      0      0      0   100%
durabletask/aio/__init__.py                                 2      0      0      0   100%
durabletask/aio/client.py                                  96      9     18      4    85%   41-43, 115-120, 136, 148->151, 157
durabletask/aio/internal/__init__.py                        0      0      0      0   100%
durabletask/aio/internal/grpc_interceptor.py               28     15      4      0    41%   34-43, 53-54, 57-58, 61-62, 65-66
durabletask/aio/internal/shared.py                         23      0     12      0   100%
durabletask/client.py                                     143     18     24      4    83%   38, 54-55, 63-64, 68, 117-119, 121, 199-204, 232->235, 241, 278-280
durabletask/deterministic.py                               81      4     10      1    95%   134-135, 153, 224
durabletask/internal/grpc_interceptor.py                   29     16      4      0    39%   41-59, 62-63, 66-67, 70-71, 74-75
durabletask/internal/helpers.py                            70      5      6      1    92%   212-214, 243->exit, 245-246
durabletask/internal/orchestrator_service_pb2.py          254    238      2      1     7%   37-274
durabletask/internal/orchestrator_service_pb2_grpc.py     239    115      2      1    52%   16-17, 20, 181-183, 188-190, 195-197, 202-204, 209-211, 216-218, 223-225, 230-232, 237-239, 244-246, 252-254, 258-260, 264-266, 270-272, 276-278, 282-284, 289-291, 296-298, 303-305, 310-312, 317-319, 324-326, 331-333, 338-340, 345-347, 352-354, 359-361, 365-505, 523, 550, 577, 604, 631, 658, 685, 712, 739, 766, 793, 820, 847, 874, 901, 928, 955, 982, 1009, 1036, 1063, 1090, 1117, 1144, 1171, 1198, 1225
durabletask/internal/shared.py                             77      4     30      5    92%   41, 46-47, 103->106, 109->114, 150
durabletask/task.py                                       266     26     68     14    88%   35, 51, 70, 81, 97, 126, 159, 177, 190, 285, 293, 310, 313, 317, 335, 339, 352, 362, 370, 409, 540, 542, 544, 546, 548, 562->559, 605
durabletask/worker.py                                     844    126    328     53    82%   247-249, 251, 283, 307-310, 324-328, 336-337, 345-347, 354-355, 371->386, 374-385, 407, 408->exit, 439-442, 445-447, 452->454, 460-485, 494, 500->502, 511-530, 549-559, 567-570, 600-603, 643, 663, 693, 710, 750, 819, 846, 849->851, 872, 901->903, 906->909, 914, 944, 959->964, 988->992, 1006, 1038, 1047-1051, 1063, 1103-1107, 1118-1122, 1125->exit, 1153, 1169, 1180-1184, 1196-1200, 1202->exit, 1230, 1243->1245, 1252->1254, 1264->1266, 1271->1273, 1284-1285, 1367-1372, 1378, 1439->1445, 1451-1453, 1458-1460, 1469, 1471, 1499->exit, 1577-1595
---------------------------------------------------------------------------------------------------
TOTAL                                                    2153    576    508     84    73%

@filintod filintod requested a review from a team as a code owner November 14, 2025 14:14
filintod
filintod commented Nov 14, 2025
View reviewed changes
Makefile
coverage-clean:
rm -f .coverage .coverage.* coverage.xml

coverage-all: coverage-clean
Copy link
Author

@filintod filintod Nov 14, 2025

Choose a reason for hiding this comment

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

could probably later be moved to tox as a new test option

Copy link
Member

@acroca acroca Nov 17, 2025

Choose a reason for hiding this comment

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

Could it be moved now? Or maybe open a separate PR for it?

@filintod
Copy link
Author

filintod commented Nov 14, 2025

@acroca ptal

filintod
filintod commented Nov 14, 2025
View reviewed changes
tests/durabletask/test_serialization.py
@@ -0,0 +1,74 @@
from collections import namedtuple
Copy link
Author

@filintod filintod Nov 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

cover paths not tested previously, and can be used in the future when we add pydantic/others

@filintod
copyright fix
4f9855d 
Signed-off-by: Filinto Duran <[email protected]>
sicoyle
sicoyle approved these changes Nov 17, 2025
View reviewed changes
Copy link

@sicoyle sicoyle left a comment

Choose a reason for hiding this comment

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

overall LGTM, few comments/questions

@filintod
feedback license
e149057 
Signed-off-by: Filinto Duran <[email protected]>
@filintod filintod force-pushed the filinto/deterministic branch from d633481 to e149057 Compare November 17, 2025 21:46
acroca
acroca reviewed Nov 18, 2025
View reviewed changes
Makefile
coverage-clean:
rm -f .coverage .coverage.* coverage.xml

coverage-all: coverage-clean
Copy link
Member

@acroca acroca Nov 17, 2025

Choose a reason for hiding this comment

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

Could it be moved now? Or maybe open a separate PR for it?

tests/durabletask/test_orchestration_e2e.py
Comment on lines +751 to +775
def test_can_add_functions_after_stop():
"""Test that orchestrators/activities can be added after stopping the worker."""

def orchestrator1(ctx: task.OrchestrationContext, _):
return "done"

def orchestrator2(ctx: task.OrchestrationContext, _):
return "done2"

def activity(ctx: task.ActivityContext, input):
return input

with worker.TaskHubGrpcWorker(stop_timeout=2.0) as w:
w.add_orchestrator(orchestrator1)
w.start()

c = client.TaskHubGrpcClient()
id = c.schedule_new_orchestration(orchestrator1)
state = c.wait_for_orchestration_completion(id, timeout=30)
assert state is not None
assert state.runtime_status == client.OrchestrationStatus.COMPLETED

# Should be able to add after stop
w.add_orchestrator(orchestrator2)
w.add_activity(activity)
Copy link
Member

@acroca acroca Nov 18, 2025

Choose a reason for hiding this comment

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

hm, what's the point of adding after stopping?

Copy link
Author

@filintod filintod Nov 18, 2025

Choose a reason for hiding this comment

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

testing that you can stop add start if wanted

@filintod @acroca
Update Makefile
930b909 
Co-authored-by: Albert Callarisa <[email protected]>
Signed-off-by: Filinto Duran <[email protected]>
@acroca acroca merged commit 16e1de1 into dapr:main Nov 18, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@acroca acroca acroca approved these changes

+1 more reviewer

@sicoyle sicoyle sicoyle approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@filintod @acroca @sicoyle