Hi Sean,
I fully agree that pytest.mark.parametrize() is cleaner and the way to
go, and I use it extensively in new tests. For what you were referring
too, this was a change in an existing test that used the old for looping
habits, so I felt it was a bit too much to ask the contributor to
refactor it.
I've tried to enhance the existing documentation page related to tests
with recommendations from this email thread and other things from
related RFCs: https://github.com/OSGeo/gdal/pull/7277
Even
Le 20/02/2023 à 16:42, Sean Gillies a écrit :
Hi all,
I just saw a maintainer recommend to a contributor that the
contributor loop over test cases from within a test function and it
prompted me to speak up about a better practice: using pytest
parameterization
https://docs.pytest.org/en/6.2.x/parametrize.html#pytest-mark-parametrize-parametrizing-test-functions.
Using a loop, like at
https://github.com/OSGeo/gdal/blob/master/autotest/gcore/gdal_stats.py#L660,
can hide bugs and add friction to development. In that particular
case, there are 5 different fill values to be checked. If there are
different code paths for each of these values (a worst case scenario)
and each has a small bug, you will have to run the whole test suite 5
times to expose the 5 bugs one at a time. Additionally, these loops
fix the order that the checks are made, and bugs can hide in test
order dependency. Ideally, GDAL's tests run in random order.
In a nutshell, you hoist the loop and list out into a test function
decorator and then pytest discovers and runs all the separate cases.
Pytest doesn't stop on the first failure in the list unless you
explicitly tell it to do so.
# Original GDAL test code
def test_fill_value():
for fill_val in [0, 1, 32767, 32768, 65535]:
func(fill_val)
assert something
# With pytest parameterization
@pytest.mark.parametrize("fill_val", [0, 1, 32767, 32768, 65535])
def test_fill_value(fill_val):
func(fill_val)
assert something
I'm not proposing rewrites of old tests, but I think it is worth
adopting a better practice for tests now and in the future.
--
Sean Gillies
_______________________________________________
gdal-dev mailing list
[email protected]
https://lists.osgeo.org/mailman/listinfo/gdal-dev
--
http://www.spatialys.com
My software is free, but my time generally not.
_______________________________________________
gdal-dev mailing list
[email protected]
https://lists.osgeo.org/mailman/listinfo/gdal-dev
