FrankYang0529 commented on code in PR #75563:
URL: https://github.com/apache/airflow/pull/75563#discussion_r4386405921


##########
ts-sdk/tests/coordinator/integration.test.ts:
##########
@@ -286,6 +286,7 @@ describe("coordinator runtime integration", () => {
       runId: "r1",
       tryNumber: 1,
       mapIndex: -1,
+      conf: {},

Review Comment:
   nit: The `conf: {}` line here passes no matter what `conf` holds. 
`toMatchObject` treats the expected `{}` as a subset of the received value, so 
`null` and `{ date: "x" }` both match `{}`. If `buildContext` used `?? null` 
instead of `?? {}`, the tests in this PR would still pass. Could you add a test 
that sends `dag_run.conf: null`?
   
   ```ts
   it("hands a handler an empty conf when the Dag run has none", async () => {
     let observedConf: unknown = undefined;
     testDag.task("read_null_conf", async () => {
       observedConf = getContext().conf;
     })();
   
     const result = await driveSupervisor(
       makeStartupDetails("read_null_conf", "test_dag", "r1", { dag_run: { 
conf: null } }),
     );
   
     expect(result.firstResponse!.body).toMatchObject({ type: "SucceedTask" });
     expect(observedConf).toEqual({});
   });
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to