uranusjr commented on a change in pull request #21264:
URL: https://github.com/apache/airflow/pull/21264#discussion_r798519843
##########
File path: tests/utils/test_trigger_rule.py
##########
@@ -18,20 +18,18 @@
import unittest
+import pytest
+
from airflow.utils.trigger_rule import TriggerRule
class TestTriggerRule(unittest.TestCase):
def test_valid_trigger_rules(self):
- assert TriggerRule.is_valid(TriggerRule.ALL_SUCCESS)
- assert TriggerRule.is_valid(TriggerRule.ALL_FAILED)
- assert TriggerRule.is_valid(TriggerRule.ALL_DONE)
- assert TriggerRule.is_valid(TriggerRule.ONE_SUCCESS)
- assert TriggerRule.is_valid(TriggerRule.ONE_FAILED)
- assert TriggerRule.is_valid(TriggerRule.NONE_FAILED)
- assert TriggerRule.is_valid(TriggerRule.NONE_FAILED_OR_SKIPPED)
- assert TriggerRule.is_valid(TriggerRule.NONE_SKIPPED)
- assert TriggerRule.is_valid(TriggerRule.DUMMY)
- assert TriggerRule.is_valid(TriggerRule.ALWAYS)
- assert TriggerRule.is_valid(TriggerRule.NONE_FAILED_MIN_ONE_SUCCESS)
- assert len(TriggerRule.all_triggers()) == 11
+ trigger_rules = TriggerRule.__members__.values()
+ for tr in trigger_rules:
+ assert isinstance(tr, str)
+ assert TriggerRule.is_valid(tr)
+ assert len(TriggerRule.all_triggers()) == len(trigger_rules)
+
+ with pytest.raises(ValueError):
+ TriggerRule("NOT_EXIST_TRIGGER_RULE")
Review comment:
I would keep the original test; the new test is basically checking
`enum`’s implementation, which is not really what we need. The original tests
is more useful because it actually checks the `TriggerRule` class is behaving
as we expect.
--
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]
