[gdal-dev] More consistent use of pytest's parameterization in autotests

[Sean Gillies]

I thought about writing something down, too, but didn't see anything about
writing tests at https://gdal.org/development/testing.html and I decided I
wasn't qualified to start a new section about testing standards.

On Mon, Feb 20, 2023, 10:52 AM Even Rouault <even.rouault at spatialys.com>
wrote:

> 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 listgdal-dev at lists.osgeo.orghttps://lists.osgeo.org/mailman/listinfo/gdal-dev
>
> -- http://www.spatialys.com
> My software is free, but my time generally not.
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/gdal-dev/attachments/20230220/13dcdc08/attachment.htm>