guan404ming commented on code in PR #62321: URL: https://github.com/apache/airflow/pull/62321#discussion_r2896487219
########## providers/imap/tests/unit/imap/hooks/test_imap.py: ########## Review Comment: Overwrite tests don't actually test overwriting. Tests like test_download_mail_attachments_with_overwrite and test_download_mail_attachments_with_overwrite_no_extension call download_mail_attachments 3 times and assert only 1 file exists, but they don't verify the content was actually overwritten with new data. Since the same payload is used each time, the test would pass even if the file was never overwritten. Consider using different payloads for subsequent calls. ########## providers/imap/tests/unit/imap/hooks/test_imap.py: ########## Review Comment: Consider using @pytest.mark.parametrize to consolidate tests. There are lots of duplication here. -- 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]
