Re: [PR] Improve code coverage for serialization [airflow]
shahar1 merged PR #51419: URL: https://github.com/apache/airflow/pull/51419 -- 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]
Re: [PR] Improve code coverage for serialization [airflow]
nailo2c commented on PR #51419: URL: https://github.com/apache/airflow/pull/51419#issuecomment-2971094948 Fixed, thanks for reviewing! If there's anything that needs improvement, please let me know 💪 -- 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]
Re: [PR] Improve code coverage for serialization [airflow]
nailo2c commented on code in PR #51419:
URL: https://github.com/apache/airflow/pull/51419#discussion_r2143148802
##
airflow-core/tests/unit/serialization/serializers/test_serializers.py:
##
@@ -166,12 +187,60 @@ def test_encode_k8s_v1pod(self):
"spec": {"containers": [{"image": "bar", "name": "foo"}]},
}
+def test_bignum(self):
+from airflow.serialization.serializers.bignum import serialize
+
+assert serialize(12345) == ("", "", 0, False)
Review Comment:
Nice suggestion! Let me fix it and check whether other test functions have
the same issue.
--
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]
Re: [PR] Improve code coverage for serialization [airflow]
uranusjr commented on code in PR #51419:
URL: https://github.com/apache/airflow/pull/51419#discussion_r2141876799
##
airflow-core/tests/unit/serialization/serializers/test_serializers.py:
##
@@ -166,12 +187,60 @@ def test_encode_k8s_v1pod(self):
"spec": {"containers": [{"image": "bar", "name": "foo"}]},
}
+def test_bignum(self):
+from airflow.serialization.serializers.bignum import serialize
+
+assert serialize(12345) == ("", "", 0, False)
Review Comment:
This should probably be named better, such as `test_bignum_not_decimal` or
something?
--
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]
Re: [PR] Improve code coverage for serialization [airflow]
uranusjr commented on code in PR #51419:
URL: https://github.com/apache/airflow/pull/51419#discussion_r2141878407
##
airflow-core/tests/unit/serialization/serializers/test_serializers.py:
##
@@ -166,12 +187,60 @@ def test_encode_k8s_v1pod(self):
"spec": {"containers": [{"image": "bar", "name": "foo"}]},
}
+def test_bignum(self):
+from airflow.serialization.serializers.bignum import serialize
+
+assert serialize(12345) == ("", "", 0, False)
+
[email protected](
+("klass", "version", "payload", "msg"),
+[
+(
+qualname(Decimal),
Review Comment:
We should use a fixed value here (and below)
--
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]
Re: [PR] Improve code coverage for serialization [airflow]
nailo2c commented on PR #51419: URL: https://github.com/apache/airflow/pull/51419#issuecomment-2957809919 Hi @shahar1, I’ve refactored the tests per your suggestion, thanks! Let me know what you think 🙏 -- 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]
Re: [PR] Improve code coverage for serialization [airflow]
nailo2c commented on PR #51419: URL: https://github.com/apache/airflow/pull/51419#issuecomment-2950515531 Thanks for the review, good point about splitting the assertions. I'll refactor the test and push an update later :) -- 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]
Re: [PR] Improve code coverage for serialization [airflow]
Copilot commented on code in PR #51419:
URL: https://github.com/apache/airflow/pull/51419#discussion_r2132617305
##
airflow-core/tests/unit/serialization/test_serialized_objects.py:
##
@@ -278,6 +303,37 @@ def __len__(self) -> int:
DAT.CONNECTION,
lambda a, b: a.get_uri() == b.get_uri(),
),
+(
+TaskCallbackRequest(
+filepath="filepath",
+ti=TI,
+bundle_name="testing",
+bundle_version=None,
+),
+DAT.TASK_CALLBACK_REQUEST,
+lambda a, b: a.ti == b.ti,
+),
+(
+DagCallbackRequest(
+filepath="filepath",
+dag_id="fake_dag",
+run_id="fake_run",
+bundle_name="testing",
+bundle_version=None,
+),
+DAT.DAG_CALLBACK_REQUEST,
+lambda a, b: a.dag_id == b.dag_id,
+),
+(Asset.ref(name="test"), DAT.ASSET_REF, lambda a, b: a.name == b.name),
+(
+DeadlineAlert(
+reference=cast("DeadlineReference",
DeadlineReference.DAGRUN_LOGICAL_DATE),
Review Comment:
Typelib.cast’s first argument should be a type, not a string. Use
cast(DeadlineReference, DeadlineReference.DAGRUN_LOGICAL_DATE) for clarity and
correct typing.
```suggestion
reference=cast(DeadlineReference,
DeadlineReference.DAGRUN_LOGICAL_DATE),
```
--
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]
Re: [PR] Improve code coverage for serialization [airflow]
nailo2c closed pull request #51419: Improve code coverage for serialization URL: https://github.com/apache/airflow/pull/51419 -- 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]
