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 <[email protected]> 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 > [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
