Rodrigo-Palma commented on code in PR #4009:
URL: https://github.com/apache/iceberg-python/pull/4009#discussion_r4085287773
##########
tests/avro/test_resolver.py:
##########
@@ -418,3 +421,30 @@ def test_writer_missing_optional_in_read_schema() -> None:
expected = StructWriter(field_writers=((None,
OptionWriter(option=StringWriter())),))
assert actual == expected
+
+
[email protected]("decoder_class", [StreamingBinaryDecoder,
CythonBinaryDecoder])
Review Comment:
Moved the list to `tests/avro/conftest.py`, with a `decoder_class` fixture
that parametrizes it, so these tests no longer carry their own copy.
I went with a fixture instead of importing the constant. CI runs `pytest
tests/avro/test_decoder.py` on its own (python-ci.yml, and the cibuildwheel
test command in the artifact workflows), and in that invocation the repository
root is not on `sys.path`, so `from tests.avro.conftest import
AVAILABLE_DECODERS` fails with `ModuleNotFoundError: No module named 'tests'`.
I hit it locally before settling on the fixture. The existing `from
tests.conftest import ...` imports have the same limitation and only work under
`python -m pytest`. A fixture needs no import, so it works under either
invocation.
`test_decoder.py` and `test_reader.py` still define their own copies. Direct
parametrization takes precedence over a fixture of the same name, so they keep
working untouched. Happy to migrate them in a follow-up if you would like: it
is 22 decorators plus one `itertools.product` call, which felt like a separate
change from this bug fix.
--
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]
