SameerMesiah97 commented on code in PR #74920:
URL: https://github.com/apache/airflow/pull/74920#discussion_r4303544498


##########
task-sdk/tests/task_sdk/bases/test_operator.py:
##########
@@ -798,6 +798,49 @@ class StringTemplateFieldsOperator(BaseOperator):
         assert len(warnings) == 1
         assert isinstance(task.template_fields, list)
 
+    def 
test_template_fields_extra_class_level_merges_into_template_fields(self):
+        """Extras declared on the class appear in ``template_fields`` after 
init."""
+
+        class ExtraTemplateOperator(BaseOperator):
+            template_fields = ("arg1",)
+            template_fields_extra = frozenset({"arg2", "arg3"})
+
+        task = ExtraTemplateOperator(task_id="op1", arg1="{{ x }}", arg2="{{ y 
}}", arg3="{{ z }}")
+
+        rendered = sorted(task.template_fields)
+        assert rendered == ["arg1", "arg2", "arg3"]

Review Comment:
   What about ordering and deduplication? I think you should add args that 
exist in both the template fields and its extra. And assert that the intended 
order is retained and duplicated args are handled i.e. only unique args exist 
in rendered.



##########
task-sdk/tests/task_sdk/bases/test_operator.py:
##########
@@ -798,6 +798,49 @@ class StringTemplateFieldsOperator(BaseOperator):
         assert len(warnings) == 1
         assert isinstance(task.template_fields, list)
 
+    def 
test_template_fields_extra_class_level_merges_into_template_fields(self):
+        """Extras declared on the class appear in ``template_fields`` after 
init."""
+
+        class ExtraTemplateOperator(BaseOperator):
+            template_fields = ("arg1",)

Review Comment:
   Does this not need an __init__? Also, since this helper class is being used 
multiple times, you could make it module level and reuse it in the tests that 
require it.



-- 
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