kaxil commented on PR #72159: URL: https://github.com/apache/airflow/pull/72159#issuecomment-5706780256
One follow-up on test strength, not on behaviour -- the approval stands and this is fine to land as is or pick up separately. [`test_notifier_template_error_fails_the_task`](https://github.com/apache/airflow/blob/5ad2e2d042d39d756756d920678be50ee9887878/providers/common/ai/tests/unit/common/ai/mixins/test_approval.py#L229-L234) passes for a reason that leaves the fix it guards unpinned. `{{ 'x' | no_such_filter }}` raises `TemplateAssertionError`, which subclasses `TemplateSyntaxError`, so narrowing the production guard to `except TemplateSyntaxError: raise` keeps every test in the file green while a DAG-bound `{{ task.bodyy }}` goes back to being swallowed, which is the behaviour this test exists to prevent. `UndefinedError` is not a `TemplateSyntaxError`, so nothing currently covers that half. The undefined-variable case is unreachable from here because the [`context` fixture](https://github.com/apache/airflow/blob/5ad2e2d042d39d756756d920678be50ee9887878/providers/common/ai/tests/unit/common/ai/mixins/test_approval.py#L96-L100) has no `dag`, so `get_template_env(dag=None)` renders non-strict, whereas a real task always renders under `DAG.template_undefined`, which defaults to `jinja2.StrictUndefined`. Putting a `dag` in the fixture and adding a `{{ task.bodyy }}` case alongside the bad-filter one would pin both halves. -- 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]
