Fokko commented on code in PR #3723:
URL: https://github.com/apache/iceberg-python/pull/3723#discussion_r3698092701


##########
tests/io/test_pyarrow.py:
##########
@@ -2512,18 +2521,11 @@ def 
test_schema_mismatch_nullability(table_schema_simple: Schema) -> None:
         )
     )
 
-    expected = """Mismatch in fields:

Review Comment:
   Why do we need to update these tests? I think this is a huge step back in 
readbility and what we're actually testing



##########
tests/io/test_pyarrow.py:
##########
@@ -170,6 +171,7 @@ def test_pyarrow_input_file() -> None:
         with pytest.raises(OSError) as exc_info:
             r.seek(0, 0)
         assert "only valid on seekable files" in str(exc_info.value)
+        r.close()

Review Comment:
   nit; I prefer using context managers similar to line 155, as it shows more 
clearly the scope of the file-input



##########
tests/conftest.py:
##########
@@ -2388,7 +2388,7 @@ def empty_home_dir_path(tmp_path_factory: 
pytest.TempPathFactory) -> str:
     return home_path
 
 
-RANDOM_LENGTH = 20
+RANDOM_LENGTH = 8

Review Comment:
   Do we know why this needed to be updated? Any limitation on Windows?



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to