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