rjgoyln commented on code in PR #74269:
URL: https://github.com/apache/airflow/pull/74269#discussion_r4194857314
##########
airflow-core/tests/unit/utils/test_file.py:
##########
@@ -72,6 +72,19 @@ def test_correct_maybe_zipped_archive(self,
mocked_is_zipfile):
assert dag_folder == "/path/to/archive.zip"
+ @mock.patch("zipfile.is_zipfile")
+ def test_correct_maybe_zipped_archive_uppercase_extension(self,
mocked_is_zipfile):
Review Comment:
Small consolidation thought: this is identical to
`test_correct_maybe_zipped_archive` above, except for the archive name.
Could we parameterize the existing test instead, so both `.zip` and `.ZIP`
stay in sync if the assertions change?
```python
@pytest.mark.parametrize("archive", ["/path/to/archive.zip",
"/path/to/archive.ZIP"])
@mock.patch("zipfile.is_zipfile")
def test_correct_maybe_zipped_archive(self, mocked_is_zipfile, archive):
...
```
I tried this locally: both cases pass with the change, and `.ZIP` fails
without it.
##########
airflow-core/tests/unit/utils/test_file.py:
##########
@@ -92,6 +105,21 @@ def test_open_maybe_zipped_archive(self, test_zip_path):
content = test_file.read()
assert isinstance(content, str)
+ def test_open_maybe_zipped_archive_uppercase_extension(self, tmp_path):
Review Comment:
Could we reuse the existing `test_zip_path` fixture here? The current test
re-implements the ZIP setup already provided by
`airflow-core/tests/conftest.py`.
We could just rename the fixture's archive to `.ZIP` and keep the test
focused on the new case:
```python
def test_open_maybe_zipped_archive_uppercase_extension(self, test_zip_path):
uppercase_zip = os.path.splitext(test_zip_path)[0] + ".ZIP"
os.rename(test_zip_path, uppercase_zip)
with open_maybe_zipped(os.path.join(uppercase_zip, "test_zip.py"), "r")
as test_file:
content = test_file.read()
assert isinstance(content, str)
```
I tried this locally, and it still fails with `NotADirectoryError` without
the regex change.
--
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]
