uranusjr commented on a change in pull request #18973:
URL: https://github.com/apache/airflow/pull/18973#discussion_r733677112
##########
File path: tests/api_connexion/schemas/test_xcom_schema.py
##########
@@ -29,169 +29,165 @@
from airflow.utils.session import create_session, provide_session
+class MockXcomCollectionItem:
+ DEFAULT_TIME = '2005-04-02T21:00:00+00:00'
+ DEFAULT_TIME_PARSED = parse_execution_date(DEFAULT_TIME)
+
+ def __init__(self):
+ self.example_ref = uuid.uuid4()
+ self.key: str = f'test_key_{self.example_ref}'
+ self.timestamp: datetime.datetime = self.DEFAULT_TIME_PARSED
+ self.execution_date: datetime.datetime = self.DEFAULT_TIME_PARSED
+ self.task_id = f'test_task_id_{self.example_ref}'
+ self.dag_id = f'test_dag_{self.example_ref}'
+
+ @property
+ def as_xcom(self):
+ return XCom(
+ key=self.key,
+ timestamp=self.timestamp,
+ execution_date=self.execution_date,
+ task_id=self.task_id,
+ dag_id=self.dag_id,
+ )
+
+ @property
+ def deserialized_xcom(self):
+ return {
+ 'key': self.key,
+ 'timestamp': self.DEFAULT_TIME,
+ 'execution_date': self.DEFAULT_TIME,
+ 'task_id': self.task_id,
+ 'dag_id': self.dag_id,
+ }
+
+ @property
+ def from_dump_xcom(self):
Review comment:
This property name feels weird to me. What does it mean semantically?
##########
File path: tests/api_connexion/schemas/test_xcom_schema.py
##########
@@ -29,169 +29,165 @@
from airflow.utils.session import create_session, provide_session
+class MockXcomCollectionItem:
+ DEFAULT_TIME = '2005-04-02T21:00:00+00:00'
+ DEFAULT_TIME_PARSED = parse_execution_date(DEFAULT_TIME)
+
+ def __init__(self):
+ self.example_ref = uuid.uuid4()
+ self.key: str = f'test_key_{self.example_ref}'
+ self.timestamp: datetime.datetime = self.DEFAULT_TIME_PARSED
+ self.execution_date: datetime.datetime = self.DEFAULT_TIME_PARSED
Review comment:
Instead of annotating these manually, let's just add annotation on
`parse_execution_date`; it'd be useful for other usages as well.
##########
File path: tests/api_connexion/schemas/test_xcom_schema.py
##########
@@ -29,169 +29,165 @@
from airflow.utils.session import create_session, provide_session
+class MockXcomCollectionItem:
+ DEFAULT_TIME = '2005-04-02T21:00:00+00:00'
+ DEFAULT_TIME_PARSED = parse_execution_date(DEFAULT_TIME)
+
+ def __init__(self):
+ self.example_ref = uuid.uuid4()
+ self.key: str = f'test_key_{self.example_ref}'
+ self.timestamp: datetime.datetime = self.DEFAULT_TIME_PARSED
+ self.execution_date: datetime.datetime = self.DEFAULT_TIME_PARSED
+ self.task_id = f'test_task_id_{self.example_ref}'
+ self.dag_id = f'test_dag_{self.example_ref}'
+
+ @property
+ def as_xcom(self):
+ return XCom(
+ key=self.key,
+ timestamp=self.timestamp,
+ execution_date=self.execution_date,
+ task_id=self.task_id,
+ dag_id=self.dag_id,
+ )
+
+ @property
+ def deserialized_xcom(self):
+ return {
+ 'key': self.key,
+ 'timestamp': self.DEFAULT_TIME,
+ 'execution_date': self.DEFAULT_TIME,
+ 'task_id': self.task_id,
+ 'dag_id': self.dag_id,
+ }
+
+ @property
+ def from_dump_xcom(self):
+ return {
+ 'key': self.key,
+ 'timestamp': self.DEFAULT_TIME_PARSED,
+ 'execution_date': self.DEFAULT_TIME_PARSED,
+ 'task_id': self.task_id,
+ 'dag_id': self.dag_id,
+ }
+
+
+class MockXcomSchema(MockXcomCollectionItem):
Review comment:
This subclass-but-replace-most-functions approach feels like an
anti-pattern. It's probably better to refactor this into something like
```python
class AbstractXComMock:
def __init__(self):
# Set most attributes...
class XComCollectionItemMock(AbstractXComMock):
# ... Implement properties...
class XComSchemaMock(AbstractXComMock):
def __init__(self):
super().__init__()
self.value = f'test_binary_{self.example_ref}'
# ... Implement properties...
```
Also both class names are very semantically very unclear to me. Please add
some comments and docstrings. Tests that are difficult for constributors to
understand are as bad as no tests at all. I actually prefer the previous
plainly written tests with a lot of duplication, since the duplicated code
express how we expect things to behave the same in various scenarios.
##########
File path: tests/api_connexion/schemas/test_xcom_schema.py
##########
@@ -29,169 +29,165 @@
from airflow.utils.session import create_session, provide_session
+class MockXcomCollectionItem:
+ DEFAULT_TIME = '2005-04-02T21:00:00+00:00'
+ DEFAULT_TIME_PARSED = parse_execution_date(DEFAULT_TIME)
+
+ def __init__(self):
+ self.example_ref = uuid.uuid4()
+ self.key: str = f'test_key_{self.example_ref}'
+ self.timestamp: datetime.datetime = self.DEFAULT_TIME_PARSED
+ self.execution_date: datetime.datetime = self.DEFAULT_TIME_PARSED
+ self.task_id = f'test_task_id_{self.example_ref}'
+ self.dag_id = f'test_dag_{self.example_ref}'
+
+ @property
+ def as_xcom(self):
+ return XCom(
+ key=self.key,
+ timestamp=self.timestamp,
+ execution_date=self.execution_date,
+ task_id=self.task_id,
+ dag_id=self.dag_id,
+ )
+
+ @property
+ def deserialized_xcom(self):
+ return {
+ 'key': self.key,
+ 'timestamp': self.DEFAULT_TIME,
+ 'execution_date': self.DEFAULT_TIME,
+ 'task_id': self.task_id,
+ 'dag_id': self.dag_id,
+ }
+
+ @property
+ def from_dump_xcom(self):
+ return {
+ 'key': self.key,
+ 'timestamp': self.DEFAULT_TIME_PARSED,
+ 'execution_date': self.DEFAULT_TIME_PARSED,
+ 'task_id': self.task_id,
+ 'dag_id': self.dag_id,
+ }
+
+
+class MockXcomSchema(MockXcomCollectionItem):
+ def __init__(self):
+ super().__init__()
+ self.value = f'test_binary_{self.example_ref}'
+
+ @property
+ def as_xcom(self):
+ return XCom(
+ key=self.key,
+ timestamp=self.timestamp,
+ execution_date=self.execution_date,
+ task_id=self.task_id,
+ dag_id=self.dag_id,
+ value=self.value.encode('ascii'),
+ )
+
+ @property
+ def deserialized_xcom(self):
+ return {
+ 'key': self.key,
+ 'timestamp': self.DEFAULT_TIME,
+ 'execution_date': self.DEFAULT_TIME,
+ 'task_id': self.task_id,
+ 'dag_id': self.dag_id,
+ 'value': self.value,
+ }
+
+ @property
+ def from_dump_xcom(self):
+ return {
+ 'key': self.key,
+ 'timestamp': self.DEFAULT_TIME_PARSED,
+ 'execution_date': self.DEFAULT_TIME_PARSED,
+ 'task_id': self.task_id,
+ 'dag_id': self.dag_id,
+ 'value': self.value,
+ }
+
+
class TestXComSchemaBase(unittest.TestCase):
def setUp(self):
- """
- Clear Hanging XComs pre test
- """
- with create_session() as session:
- session.query(XCom).delete()
+ self.clear_db()
def tearDown(self) -> None:
+ self.clear_db()
+
+ @staticmethod
+ def clear_db():
"""
- Clear Hanging XComs post test
+ Clear Hanging XComs pre test
"""
Review comment:
This is actually called both before and after tests, so both the old and
new docstrings are inaccurate.
--
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]
