o-nikolas commented on code in PR #35226:
URL: https://github.com/apache/airflow/pull/35226#discussion_r1378245972


##########
tests/providers/amazon/aws/sensors/test_batch.py:
##########
@@ -29,59 +30,86 @@
 )
 from airflow.providers.amazon.aws.triggers.batch import BatchJobTrigger
 
+if TYPE_CHECKING:
+    from airflow.providers.amazon.aws.sensors.base_aws import AwsBaseSensor
+
+
 TASK_ID = "batch_job_sensor"
 JOB_ID = "8222a1c2-b246-4e19-b1b8-0039bb4407c0"
 AWS_REGION = "eu-west-1"
 ENVIRONMENT_NAME = "environment_name"
 JOB_QUEUE = "job_queue"
 
-
-@pytest.fixture(scope="module")
-def batch_sensor() -> BatchSensor:
-    return BatchSensor(
-        task_id="batch_job_sensor",
-        job_id=JOB_ID,
-    )
+SOFT_FAIL_CASES = [
+    pytest.param(False, AirflowException, id="not-soft-fail"),
+    pytest.param(True, AirflowSkipException, id="soft-fail"),
+]
 
 
 @pytest.fixture(scope="module")
 def deferrable_batch_sensor() -> BatchSensor:
     return BatchSensor(task_id="task", job_id=JOB_ID, region_name=AWS_REGION, 
deferrable=True)
 
 
-class TestBatchSensor:
-    @mock.patch.object(BatchClientHook, "get_job_description")
-    def test_poke_on_success_state(self, mock_get_job_description, 
batch_sensor: BatchSensor):
-        mock_get_job_description.return_value = {"status": "SUCCEEDED"}
-        assert batch_sensor.poke({}) is True
-        mock_get_job_description.assert_called_once_with(JOB_ID)
+@pytest.fixture
+def mock_get_job_description():
+    with mock.patch.object(BatchClientHook, "get_job_description") as m:
+        yield m
 
-    @mock.patch.object(BatchClientHook, "get_job_description")
-    def test_poke_on_failure_state(self, mock_get_job_description, 
batch_sensor: BatchSensor):
-        mock_get_job_description.return_value = {"status": "FAILED"}
-        with pytest.raises(AirflowException, match="Batch sensor failed. AWS 
Batch job status: FAILED"):
-            batch_sensor.poke({})
 
-        mock_get_job_description.assert_called_once_with(JOB_ID)
+@pytest.fixture
+def mock_batch_client():
+    with mock.patch.object(BatchClientHook, "client") as m:
+        yield m
+
+
+class BaseBatchSensorsTests:
+    """Base test class for Batch Sensors."""
+
+    op_class: type[AwsBaseSensor]
+    default_op_kwargs: dict[str, Any]
+
+    def test_base_aws_op_attributes(self):
+        op = self.op_class(**self.default_op_kwargs)
+        assert op.hook.aws_conn_id == "aws_default"
+        assert op.hook._region_name is None
+        assert op.hook._verify is None
+        assert op.hook._config is None
+
+        op = self.op_class(
+            **self.default_op_kwargs,
+            aws_conn_id="aws-test-custom-conn",
+            region_name="eu-west-1",
+            verify=False,
+            botocore_config={"read_timeout": 42},
+        )
+        assert op.hook.aws_conn_id == "aws-test-custom-conn"
+        assert op.hook._region_name == "eu-west-1"
+        assert op.hook._verify is False
+        assert op.hook._config is not None
+        assert op.hook._config.read_timeout == 42
 
-    @mock.patch.object(BatchClientHook, "get_job_description")
-    def test_poke_on_invalid_state(self, mock_get_job_description, 
batch_sensor: BatchSensor):
-        mock_get_job_description.return_value = {"status": "INVALID"}
-        with pytest.raises(
-            AirflowException, match="Batch sensor failed. Unknown AWS Batch 
job status: INVALID"
-        ):
-            batch_sensor.poke({})
 
+class TestBatchSensor(BaseBatchSensorsTests):
+    op_class = BatchSensor
+
+    @pytest.fixture(autouse=True)
+    def setup_test_cases(self):
+        self.default_op_kwargs = dict(
+            task_id="test_batch_sensor",
+            job_id=JOB_ID,
+        )

Review Comment:
   The literal is slightly faster I believe (not the most compelling reason in 
this case, but worth noting since we're talking about it) but the more 
important thing is the keys are quoted in the literal. That way you don't have 
to worry about keywords or special characters in the key name. Which isn't a 
worry in this case, but I think it's best to choose one and stay consistent 
across the code base. So I'd also vote to move this to the literal `{...}`



-- 
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: commits-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to