<div dir="auto"><div>I thought about writing something down, too, but didn't see anything about writing tests at <a href="https://gdal.org/development/testing.html" target="_blank" rel="noreferrer">https://gdal.org/development/testing.html</a> and I decided I wasn't qualified to start a new section about testing standards.<br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Feb 20, 2023, 10:52 AM Even Rouault <<a href="mailto:even.rouault@spatialys.com" rel="noreferrer noreferrer" target="_blank">even.rouault@spatialys.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
  
    
  
  <div>
    <p>Hi Sean,</p>
    <p>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.</p>
    <p>I've tried to enhance the existing documentation page related to
      tests with recommendations from this email thread and other things
      from related RFCs: <a href="https://github.com/OSGeo/gdal/pull/7277" rel="noreferrer noreferrer noreferrer" target="_blank">https://github.com/OSGeo/gdal/pull/7277</a> <br>
    </p>
    <p>Even<br>
    </p>
    <div>Le 20/02/2023 à 16:42, Sean Gillies a
      écrit :<br>
    </div>
    <blockquote type="cite">
      
      <div dir="ltr">
        <div>Hi all,</div>
        <div><br>
        </div>
        <div>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 <a href="https://docs.pytest.org/en/6.2.x/parametrize.html#pytest-mark-parametrize-parametrizing-test-functions" rel="noreferrer noreferrer noreferrer" target="_blank">https://docs.pytest.org/en/6.2.x/parametrize.html#pytest-mark-parametrize-parametrizing-test-functions</a>.<br>
        </div>
        <div><br>
        </div>
        <div>Using a loop, like at <a href="https://github.com/OSGeo/gdal/blob/master/autotest/gcore/gdal_stats.py#L660" rel="noreferrer noreferrer noreferrer" target="_blank">https://github.com/OSGeo/gdal/blob/master/autotest/gcore/gdal_stats.py#L660</a>,
          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.</div>
        <div><br>
        </div>
        <div>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.<br>
        </div>
        <div><br>
        </div>
        <div># Original GDAL test code<br>
        </div>
        <div>def test_fill_value():<br>
        </div>
        <div><span>    for</span> <span>fill_val</span> <span>in</span> [<span>0</span>,
          <span>1</span>, <span>32767</span>,
          <span>32768</span>, <span>65535</span>]:</div>
        <div>        func(fill_val)<br>
        </div>
        <div>        assert something<br>
        </div>
        <div><br>
        </div>
        <div># With pytest parameterization<br>
        </div>
        <div>@pytest.mark.parametrize("fill_val", [0, 1, 32767, 32768,
          65535])</div>
        <div>
          <div>def test_fill_value(fill_val):</div>
          <div>        func(fill_val)<br>
          </div>
          <div>        assert something</div>
        </div>
        <div><br>
        </div>
        <div>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.</div>
        <div><br>
        </div>
        -- <br>
        <div dir="ltr" data-smartmail="gmail_signature">Sean Gillies</div>
      </div>
      <br>
      <fieldset></fieldset>
      <pre>_______________________________________________
gdal-dev mailing list
<a href="mailto:gdal-dev@lists.osgeo.org" rel="noreferrer noreferrer noreferrer" target="_blank">gdal-dev@lists.osgeo.org</a>
<a href="https://lists.osgeo.org/mailman/listinfo/gdal-dev" rel="noreferrer noreferrer noreferrer" target="_blank">https://lists.osgeo.org/mailman/listinfo/gdal-dev</a>
</pre>
    </blockquote>
    <pre cols="72">-- 
<a href="http://www.spatialys.com" rel="noreferrer noreferrer noreferrer" target="_blank">http://www.spatialys.com</a>
My software is free, but my time generally not.</pre>
  </div>

</blockquote></div></div></div>