feluelle commented on a change in pull request #6352: [AIRFLOW-5683] Add 
propagate_skipped_state to SubDagOperator
URL: https://github.com/apache/airflow/pull/6352#discussion_r341123900
 
 

 ##########
 File path: tests/operators/test_subdag_operator.py
 ##########
 @@ -237,3 +240,65 @@ def test_rerun_failed_subdag(self):
 
         sub_dagrun.refresh_from_db()
         self.assertEqual(sub_dagrun.state, State.RUNNING)
+
+    @parameterized.expand([
+        (SkippedStatePropagationOptions.ALL_LEAVES, [State.SKIPPED, 
State.SKIPPED], True),
+        (SkippedStatePropagationOptions.ALL_LEAVES, [State.SKIPPED, 
State.SUCCESS], False),
+        (SkippedStatePropagationOptions.ANY_LEAF, [State.SKIPPED, 
State.SUCCESS], True),
+        (SkippedStatePropagationOptions.ANY_LEAF, [State.FAILED, 
State.SKIPPED], True),
 
 Review comment:
   I will add this one:
   ```python
   (None, [State.SKIPPED, State.SKIPPED], False),
   ```
   
   I am only testing for dag_run's which succeeded.
   
   So 
   ```python
   (None, [State.FAILED, State. FAILED], False),
   ```
   should not be possible in a succeeded dag run, right?
   
   Failed dag_run's will raise an exception before even coming to the new 
functionality I added.
   ```python
   if dag_run.state != State.SUCCESS:
       raise AirflowException(
           "Expected state: SUCCESS. Actual state: {}".format(dag_run.state)
       )
   ```
   
   Should I also test for failed dag_run's ?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to