<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<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 class="moz-txt-link-freetext" href="https://github.com/OSGeo/gdal/pull/7277">https://github.com/OSGeo/gdal/pull/7277</a> <br>
</p>
<p>Even<br>
</p>
<div class="moz-cite-prefix">Le 20/02/2023 à 16:42, Sean Gillies a
écrit :<br>
</div>
<blockquote type="cite"
cite="mid:CAOodmJqJQr0oHkpPK=jsDgfu6S3kQVDsqM298S2Ei31zq+Ka1g@mail.gmail.com">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<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"
moz-do-not-send="true" class="moz-txt-link-freetext">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"
moz-do-not-send="true" class="moz-txt-link-freetext">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 class="gmail-pl-k"> for</span> <span
class="gmail-pl-s1">fill_val</span> <span
class="gmail-pl-c1">in</span> [<span class="gmail-pl-c1">0</span>,
<span class="gmail-pl-c1">1</span>, <span class="gmail-pl-c1">32767</span>,
<span class="gmail-pl-c1">32768</span>, <span
class="gmail-pl-c1">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" class="gmail_signature"
data-smartmail="gmail_signature">Sean Gillies</div>
</div>
<br>
<fieldset class="moz-mime-attachment-header"></fieldset>
<pre class="moz-quote-pre" wrap="">_______________________________________________
gdal-dev mailing list
<a class="moz-txt-link-abbreviated moz-txt-link-freetext" href="mailto:gdal-dev@lists.osgeo.org">gdal-dev@lists.osgeo.org</a>
<a class="moz-txt-link-freetext" href="https://lists.osgeo.org/mailman/listinfo/gdal-dev">https://lists.osgeo.org/mailman/listinfo/gdal-dev</a>
</pre>
</blockquote>
<pre class="moz-signature" cols="72">--
<a class="moz-txt-link-freetext" href="http://www.spatialys.com">http://www.spatialys.com</a>
My software is free, but my time generally not.</pre>
</body>
</html>