jason810496 commented on code in PR #73648:
URL: https://github.com/apache/airflow/pull/73648#discussion_r4115691866
##########
airflow-e2e-tests/tests/airflow_e2e_tests/go_sdk_tests/test_go_sdk_taskflow_binding.py:
##########
@@ -146,21 +162,30 @@ def
test_via_struct_default_arg_tolerates_unclaimed_default(completed_run: _Comp
assert completed_run.xcom("via_struct_default_arg") == {"region":
"eu-west-1"}
-def test_via_struct_more_args_runs_anyway(completed_run: _CompletedRun):
+def test_via_struct_more_args_warns_and_runs(completed_run: _CompletedRun):
"""The call passes ``unused_label``, which the Go struct does not declare.
Name
binding cannot shift, so the extra argument is warned about rather than
failing
the task, and everything the struct does declare still binds."""
assert completed_run.xcom("via_struct_more_args") == {"region":
"eu-west-1"}
+ logs = completed_run.logs("via_struct_more_args")
+ assert "Dag's call passed argument(s) the task handler does not declare"
in logs, logs
+ assert "unused_label" in logs, logs
-def test_via_struct_fewer_args_runs_anyway(completed_run: _CompletedRun):
+def test_via_struct_fewer_args_warns_and_runs(completed_run: _CompletedRun):
"""The Go struct declares ``not_in_dag``, which the stub has no parameter
for. The
field keeps its Go zero value and the mismatch is warned about rather than
failing
- the task, so the two sides can drift without breaking the Dag."""
+ the task, so the two sides can drift without breaking the Dag.
+
+ The warning is the assertion that matters: the zero-valued field alone
would look
+ the same as the older behaviour that filled it silently."""
assert completed_run.xcom("via_struct_fewer_args") == {
"region": "eu-west-1",
"not_in_dag_was_empty": True,
}
+ logs = completed_run.logs("via_struct_fewer_args")
+ assert "Task handler declares argument(s) the Dag's call did not pass" in
logs, logs
+ assert "not_in_dag" in logs, logs
Review Comment:
Good catch, thanks. Added `log_records()` and a `warning()` helper, and both
tests now assert on the record field (`declared_not_passed` /
`passed_not_declared`) instead of the joined text. Fixed in 3532d9a2be.
##########
go-sdk/pkg/binding/binding_test.go:
##########
@@ -925,14 +925,17 @@ func (s *BindingSuite)
TestResolveStructUnclaimedFromDefaultStaysSilent() {
s.NotContains(logs, "the task handler does not declare")
}
-func (s *BindingSuite) TestResolveStructEmptySpecFailsLoudly() {
+func (s *BindingSuite) TestResolveStructEmptySpecWarns() {
+ // An argless call sends no spec at all, so this is the ordinary shape
of a
+ // stub called as `my_task()`, not a sign of an Airflow that cannot
send one.
fn := func(actx contexttest.Context, input simpleInput) error { return
nil }
for name, args := range map[string][]Arg{"nil-spec": nil, "empty-spec":
{}} {
s.Run(name, func() {
- _, err := s.resolve(fn, args, &fakeXComClient{})
- if s.Assert().Error(err) {
- s.Contains(err.Error(), "no TaskFlow arg
bindings arrived")
- }
+ got, logs, err := s.resolveWithLogs(fn, args,
&fakeXComClient{})
+ s.Require().NoError(err)
Review Comment:
Right, the description was stale after the last commit. Updated: an empty
spec is now warned about and the task runs, since that is what an argless call
sends.
--
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]
