jayamanikharyono commented on code in PR #69382:
URL: https://github.com/apache/airflow/pull/69382#discussion_r3612111977


##########
providers/tableau/tests/unit/tableau/operators/test_tableau.py:
##########
@@ -530,3 +535,122 @@ def 
test_blocking_refresh_forwards_wait_for_state_options(self, mock_tableau_hoo
             exponential_backoff=True,
             max_check_interval=120,
         )
+
+    @patch("airflow.providers.tableau.operators.tableau.TableauHook")
+    def test_skip_on_conflict_skips_task(self, mock_tableau_hook):
+        """
+        Test that a 409093 Resource Conflict is turned into an 
AirflowSkipException
+        when skip_on_conflict=True.
+        """
+        mock_tableau_hook.get_all = Mock(return_value=self.mock_datasources)
+        mock_tableau_hook.return_value.__enter__ = 
Mock(return_value=mock_tableau_hook)
+        mock_tableau_hook.server.datasources.refresh.side_effect = 
ServerResponseError(
+            "409093", "Resource Conflict", "Job is already queued. Not queuing 
a duplicate."
+        )
+
+        operator = TableauOperator(
+            find="ds_2",
+            resource="datasources",
+            skip_on_conflict=True,
+            **self.kwargs,
+        )
+
+        with pytest.raises(AirflowSkipException):
+            operator.execute(context={})
+
+    @patch("airflow.providers.tableau.operators.tableau.TableauHook")
+    def test_conflict_not_skipped_by_default(self, mock_tableau_hook):
+        """
+        Test that a 409093 Resource Conflict still fails the task when 
skip_on_conflict
+        is left at its default (False), preserving pre-existing behavior.
+        """
+        mock_tableau_hook.get_all = Mock(return_value=self.mock_datasources)
+        mock_tableau_hook.return_value.__enter__ = 
Mock(return_value=mock_tableau_hook)
+        mock_tableau_hook.server.datasources.refresh.side_effect = 
ServerResponseError(
+            "409093", "Resource Conflict", "Job is already queued. Not queuing 
a duplicate."
+        )
+
+        operator = TableauOperator(
+            find="ds_2",
+            resource="datasources",
+            **self.kwargs,
+        )
+
+        with pytest.raises(ServerResponseError):
+            operator.execute(context={})
+
+    @patch("airflow.providers.tableau.operators.tableau.TableauHook")
+    def test_skip_on_conflict_does_not_swallow_other_server_errors(self, 
mock_tableau_hook):
+        """
+        Test that skip_on_conflict only catches the 409093 conflict code; other
+        ServerResponseErrors still fail the task.
+        """
+        mock_tableau_hook.get_all = Mock(return_value=self.mock_datasources)
+        mock_tableau_hook.return_value.__enter__ = 
Mock(return_value=mock_tableau_hook)
+        mock_tableau_hook.server.datasources.refresh.side_effect = 
ServerResponseError(
+            "500000", "Internal Server Error", "Something else went wrong."
+        )
+
+        operator = TableauOperator(
+            find="ds_2",
+            resource="datasources",
+            skip_on_conflict=True,
+            **self.kwargs,
+        )
+
+        with pytest.raises(ServerResponseError):
+            operator.execute(context={})
+
+    @patch("airflow.providers.tableau.operators.tableau.TableauHook")
+    def test_skip_on_conflict_skips_task_run(self, mock_tableau_hook):

Review Comment:
   The difference between these two tests is the Tableau API operation being 
triggered:
   
   The first test covers triggering a **[Tableau Resource 
Refresh](https://github.com/jayamanikharyono/airflow/blob/1f57d4e855b9f91876af39c1275a7280d4883d1a/providers/tableau/tests/unit/tableau/operators/test_tableau.py#L539-L559)**
   ```python
   def test_resource_refresh_skip_on_conflict_raises_skip_exception(self, 
mock_tableau_hook):
           """
           Test that a 409093 Resource Conflict on a Tableau resource refresh 
is turned into an
           AirflowSkipException when skip_on_conflict=True.
           """
   ``` 
   
   The second test covers triggering a **[Tableau Task 
Run](https://github.com/jayamanikharyono/airflow/blob/1f57d4e855b9f91876af39c1275a7280d4883d1a/providers/tableau/tests/unit/tableau/operators/test_tableau.py#L627-L642)**
   ```python
   def test_task_run_skip_on_conflict_raises_skip_exception(self, 
mock_tableau_hook):
           """
           Test that a 409093 Resource Conflict on a Tableau task run is turned 
into an
           AirflowSkipException when skip_on_conflict=True.
           """
   ```
   
   
   I've updated the test method names and docstrings to make the distinction 
between these two cases clearer.



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