Repository navigation
test(otel): cover replay and chained execution - #796
zhongkechen wants to merge 8 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| if error.message != HELPER_FAILURE: | ||
| raise | ||
| else: | ||
| raise RuntimeError("Expected the intentional helper failure") |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| ("Otel21CompletedStepReplay", "otel-invocation-21"), | ||
| ("Otel22UserFunctionContext", "otel-invocation-22"), | ||
| ("Otel23CallbackFunctionContext", "otel-invocation-23"), | ||
| ("Otel24InvocationRetryStatus", "otel-invocation-24"), | ||
| ("Otel27ChainedInvokeSuspend", "otel-invocation-27"), |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment has been minimized.
This comment has been minimized.
| BuildMethod: makefile | ||
| TestingMetadata: | ||
| TestDescription: | ||
| - otel-execution-21 |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_iqv2alr2znzezrkkaokhczcccj
[P2] Resolve the known history-order failure before enabling case 21
This activates case 21 in execution-view CI despite the unchanged failure documented in the PR: WaitSucceeded preceded InvocationCompleted by 63 ms. The wait timer runs independently of invocation cleanup, so that ordering can legitimately occur while the execution still returns before-after. The shared requirement's fixed ordering therefore produces false failures for valid SDK behavior. Land a coordinated requirement correction that accepts both valid orderings while preserving the replay assertions, or retain the pending declaration until that correction lands.
To suppress repeats, reply /ai verdict deferred, /ai verdict false-positive, or /ai verdict out-of-scope on the first nonblank line. Add rationale on later lines. A new /ai verdict accepted allows future repeats again.
| if error.message != HELPER_FAILURE: | ||
| raise | ||
| else: | ||
| raise RuntimeError("Expected the intentional helper failure") |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_qhb22zbhwqp6f4uxhumsw3g7n3
[P2] Return the untested error guards to the design phase
Rule 4, “Every branch has a test that fails without it,” remains unsatisfied. Both test parametrizations use the helper that raises HELPER_FAILURE; neither exercises a different error message or an unexpected successful return. Removing either defensive rejection leaves the test results unchanged. Return to the design phase under rule 8 and decide whether these cases need meaningful coverage or whether the fixed fixture permits a simpler approach.
To suppress repeats, reply /ai verdict deferred, /ai verdict false-positive, or /ai verdict out-of-scope on the first nonblank line. Add rationale on later lines. A new /ai verdict accepted allows future repeats again.
| barriers: dict[str, Barrier] = {} | ||
| for pair in ( | ||
| ("parallel-a", "parallel-b"), | ||
| ("parallel-step-a", "parallel-step-b"), | ||
| ("map-0", "map-1"), | ||
| ("map-step-0", "map-step-1"), |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_zhaqzag3s27vy6z7ak6uvvprpg
[P3] Document simpler alternatives for the added harness
Rule 2, “Simplify before you add,” requires the description to identify simpler options considered and explain why they do not work when logic is added. This introduces a label-indexed barrier harness and callback wrapper, but the description lists coverage and validation without that comparison. Existing OTel e2e tests already exercise branch isolation and replay, making the boundary between extending existing coverage and adding this harness worth establishing. Return to the design phase under rule 8 and record the alternatives and the reason for the chosen minimum approach.
To suppress repeats, reply /ai verdict deferred, /ai verdict false-positive, or /ai verdict out-of-scope on the first nonblank line. Add rationale on later lines. A new /ai verdict accepted allows future repeats again.
Codex AI reviewFound one CI issue and two approach issues:
Review was read-only; tests were inspected but not executed. Reviewed commit |
Add the remaining OpenTelemetry conformance coverage through the public Core 3 / OTel 2 APIs: skip completed step bodies (21), retain user-function and callback tracing contexts (22–23), report invocation retries as RETRYING/UNSET (24), and suspend/resume a chained child through a durable wait (27).
Merge order: #864 first, then this PR. Case 11's callback/replay fix and regression test are now owned by #864. This PR is stacked on
fix/otel11-replay-barrier, so its review diff excludes that fix. After #864 merges, retarget this PR tomain.The shared OpenTelemetry workflow tracks the conformance repository's
mainbranch. The test catalog defaults tomainfor pull requests, pushes, and manual runs; manual runs retain theconformance_test_refoverride. SDK handlers run from the commit under test.Coverage and follow-up
This PR supplies handlers for cases 21–24 and 27, replacing their pending-coverage declarations from #864. Cases 25–26 remain explicitly
NOT_IMPLEMENTEDin both telemetry views because they depend on separate runtime/testing fixes. The existing case 17 handler remains active; its additional callback-error regression is deferred.The deferred source is available in 6484e577.
Before restoring those tests, correct their history expectations: included diagnostic payloads are untruncated; metadata-only history omits payloads and cannot reconstruct error details. The empty
Error.Payloadservice shape is unverified against deployed AWS Lambda. Preserve case 26's assertion inside the observed step that completion has already reached the exporter, plus raw export counts at both later barriers.The shared orchestrator runs invocation and execution suites. It no longer schedules the legacy long-running suite; existing long-running templates and local contract tests remain.
Validation
After splitting out #864, on Python 3.13.17:
hatch run test:all packages/aws-durable-execution-sdk-python-conformance-tests-otel/tests -q --tb=short: 75 passed.git diff --check: passed.The latest cloud run at
f56cf00bpassed three of four telemetry jobs. The S3 execution-view job failed case 21 becauseWaitSucceededprecededInvocationCompletedby 63 ms, opposite to its fixed history expectation. The execution succeeded and returnedbefore-after; that timing issue is unchanged by this split. The new standalone PR does not include case 21.Existing test expectation changes
test_templates_map_every_otel_requirement,test_templates_declare_only_the_expected_functions,test_function_names_identify_the_requirement,test_makefile_builds_every_function,test_every_registered_handler_exists,test_handler_modules_are_exactly_the_expected_set, andtest_chained_invoke_functions_point_at_their_target: extend the expected mappings, modules, functions, and targets for cases 21–24 and 27.test_deferred_requirements_are_explicit: reduce pending cases from 21–27 to 25–26 because this PR supplies the other handlers, retaining issue references and complete catalog accounting.test_opentelemetry_conformance_caller_uses_current_workflow_contract: expect the shared workflow and catalog to usemain, matching the requested default.Case 27 companion PRs: JS #1068 and Java #824.