<div dir="ltr"><div>Hi all,</div><div><br></div>Plethora of additional points in support of pytest:<div><br></div><div>- Classification: bootstrap dials, then run the tests. All will say &quot;OK&quot; but in reality only about 1/3 of them ran for various skip reasons. This is *horribly* misleading and has led to broken builds in the past. I proposed a patch a while ago to add a skip classification but this was rejected because some tests (two, I think?) in phenix classified as &#39;SKIP&#39; instead of &#39;OK&#39;. There is a system to classify as &#39;WARN&#39;, but *only* if phenix_regression is installed, it misclassifies several items in dials/cctbx (apparently OK showing as WARN is acceptable where OK as SKIP wasn&#39;t) and in any case relies on a horrible whitelist of &quot;text that looks like warnings but shouldn&#39;t classify the test as a warning&quot;. In pytest, things like skip/warn/expected failure are easy (e.g. you can mark tests as &quot;this is currently expected to fail&quot; rather than leaving your CI^H^Hdaily build failing permanently)</div><div>- Technical debt/NIH: It&#39;s one framework for you to learn, sure. But it&#39;s N &#39;frameworks&#39; to learn for the next N developers - and since pytest is very common, there is a good chance that they&#39;ll either know it already, or the knowledge will be usefully applicable to other projects in the future. Every single new developer has to go through the WTF-laden process of learning the &#39;historical&#39; idiosyncrasies.</div><div>

<span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">- Categorisation: A dumb &#39;list of tests&#39; is okay until you have e.g. very long regression tests that don&#39;t need to be run as frequently as unit tests</span><br></div><div>- Documentation: How does something work/how do you do something unfamiliar? I&#39;ll let you compare the documentation for libtbx approx_equal <a href="https://github.com/cctbx/cctbx_project/blob/master/libtbx/test_utils/__init__.py#L281">https://github.com/cctbx/cctbx_project/blob/master/libtbx/test_utils/__init__.py#L281</a> and pytest&#39;s approx <a href="https://docs.pytest.org/en/latest/builtin.html#pytest.approx">https://docs.pytest.org/en/latest/builtin.html#pytest.approx</a></div><div>- Maintenance/bit rot: Somebody else writes this documentation, somebody else finds and fixes the bugs, and somebody else is responsible for making things better. Something untouched for a decade that people are afraid to or prevented from improving is not a good thing.</div><div><br></div><div>Additionally, I&#39;d want to argue great improvements and benefits in things like locality, parametrization, granularity (separation of tests), formality, discoverability, as well as lots of other things, but I think you get my point. Basically, every possible improvement a formal testing suite could have over what is effectively a dumb list of files to run.</div><div><br></div><div>Python&#39;s unittest would have most of these benefits, but with the disadvantage of a little more boilerplate. Pytest has less boilerplate and syntax, and for almost equivalent total functionality you only have to remember a) &quot;call the files test_something.py and the functions test_something&quot; b) use assert (you can even use libtbx.approx_equal if you want!).</div><div><br></div><div>Nick</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Mar 3, 2018 at 11:03 AM,  <span dir="ltr">&lt;<a href="mailto:markus.gerstel@diamond.ac.uk" target="_blank">markus.gerstel@diamond.ac.uk</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Pavel,<br>
<br>
pytest was suggested* by a couple of people in 2015 when I asked about unittest. Your recommendation at that time was to go ahead, so thats what I did - with the result that in April 2016 the pytest compatibility layer was introduced to cctbx and pytest included in the base build, so we can use pytests alongside libtbx tests. In our packages we have been using pytests for well over 2 years now, and just one month ago converted xia2 to pytest completely.<br>
<br>
As to &quot;why pytest&quot; I would like to point you to our wiki page &quot;Why pytest?&quot; at <a href="https://github.com/dials/dials/wiki/Why-pytest%3F" rel="noreferrer" target="_blank">https://github.com/dials/<wbr>dials/wiki/Why-pytest%3F</a> which does make the case in more detail and includes instructions to enable pytest for cctbx modules. However I&#39;m more than happy to demonstrate part of it here using your example.<br>
<br>
First of all, the example you gave is not quite correct. You are missing the import for approx_equal and missed the crucial line in run_tests.py that ensures the test is actually run. This is one of the drawbacks of libtbx testing: there is no automatic test discovery. And this tends to happen quite a lot, cf. <a href="https://github.com/dials/dials/issues/506" rel="noreferrer" target="_blank">https://github.com/dials/<wbr>dials/issues/506</a><br>
<br>
In pytest your example would look like this:<br>
<br>
import pytest<br>
def test_that_2_times_2_is_4():<br>
  x = 2<br>
  result = x*x<br>
  assert result == pytest.approx(4)<br>
<br>
which I would argue has the exact same documentation/example value.<br>
<br>
<br>
Let&#39;s change the expectation value from 4 to 5, forget everything we know about the code, run it and compare the output:<br>
<br>
<br>
$ python tst_multiply.py<br>
approx_equal eps: 1e-06<br>
approx_equal multiplier: 10000000000.0<br>
4.0 approx_equal ERROR<br>
5.0 approx_equal ERROR<br>
<br>
Traceback (most recent call last):<br>
  File &quot;tst_multiply.py&quot;, line 10, in &lt;module&gt;<br>
    exercise()<br>
  File &quot;tst_multiply.py&quot;, line 7, in exercise<br>
    assert approx_equal(result, 5., 1.e-6)<br>
AssertionError<br>
<br>
*Something* went wrong, it has to do with 1e-06, a very large number (I still don&#39;t understand what that means), two errors for 4.0 and 5.0, and a function called exercise(), which tells me exactly nothing. I have to look into the code to understand what went wrong and why. From the outset I don&#39;t know what the code is supposed to do.<br>
<br>
<br>
$ pytest<br>
==============================<wbr>===== test session starts ==============================<wbr>=====<br>
platform linux2 -- Python 2.7.13, pytest-3.1.3, py-1.4.34, pluggy-0.4.0<br>
rootdir: /dls/tmp/wra62962/directories/<wbr>lh0UFeF6, inifile:<br>
plugins: xdist-1.20.1, timeout-1.2.0, forked-0.2, cov-2.5.1<br>
collected 2 items<br>
<br>
test_multiplication.py .F<br>
<br>
==============================<wbr>========== FAILURES ==============================<wbr>===========<br>
___________________________ test_that_two_times_two_<wbr>equals_five ___________________________<br>
<br>
    def test_that_two_times_two_<wbr>equals_five():<br>
      x = 2<br>
      result = x*x<br>
&gt;     assert result == pytest.approx(5)<br>
E     AssertionError: assert 4 == 5 +- 5.0e-06<br>
E      +  where 5 +- 5.0e-06 = &lt;class &#39;_pytest.python.approx&#39;&gt;(5)<br>
E      +    where &lt;class &#39;_pytest.python.approx&#39;&gt; = pytest.approx<br>
<br>
test_multiplication.py:11: AssertionError<br>
=========================== 1 failed, 1 passed in 0.04 seconds ============================<br>
<br>
<br>
Oh look, a test to ensure two times two equals five failed. Apparently it compared 4 to 5 +- 5e-06.<br>
You also see that the other test in the file that I left in (which compares 2*2 to 4) worked. You didn&#39;t know that from the libtbx-style test output.<br>
<br>
Now I can fix the test, and pytest allows me to just rerun the failed tests, not everything.<br>
If you want to know how, have a look at <a href="https://github.com/dials/dials/wiki/pytest" rel="noreferrer" target="_blank">https://github.com/dials/<wbr>dials/wiki/pytest</a> which contains a lot more information about running pytest and converting tests.<br>
<br>
If you don&#39;t want to use it that is fine, too. Thanks to the compatibility layer you can still use libtbx.run_tests_parallel, and you will still get the more useful assertion messages. I would encourage you to try it though. You might find it useful.<br>
<br>
-Markus<br>
<br>
<br>
* And rightly so; pytest requires much less boilerplate, produces cleaner code, and is overall just better - Thank you, Luc. Thank you, Jamasi.<br>
PS: On fable specifically: thanks to the conversion I already found and fixed one broken test which didn&#39;t fail, and another test with race conditions.<br>
<br>
______________________________<wbr>__________<br>
From: <a href="mailto:cctbxbb-bounces@phenix-online.org">cctbxbb-bounces@phenix-online.<wbr>org</a> [<a href="mailto:cctbxbb-bounces@phenix-online.org">cctbxbb-bounces@phenix-<wbr>online.org</a>] on behalf of Pavel Afonine [<a href="mailto:pafonine@lbl.gov">pafonine@lbl.gov</a>]<br>
Sent: Saturday, March 03, 2018 07:13<br>
To: cctbx mailing list; Winter, Graeme (DLSLtd,RAL,LSCI)<br>
Subject: Re: [cctbxbb] [git/cctbx] master: rename test files, remove them from run_tests (23a4a6fe4)<br>
<div class="HOEnZb"><div class="h5"><br>
I&#39;d say at least because:<br>
<br>
- the first 10+ years of CCTBX did not use pytest. AFAIK, the first<br>
attempt was by our postdoc Youval Dar back in 2015 (correct me if I&#39;m<br>
wrong). I feel adding different testing styles are only to make the<br>
code-base inconsistent (very much like mixing flex and np arrays isn&#39;t<br>
cool, in my opinion!).<br>
<br>
- originally tests were considered as simple usage examples for<br>
functionalities they are testing; this is because writing and (most<br>
importantly!) maintaining the proper documentation was not provisioned.<br>
A simple test like<br>
<br>
def exercise():<br>
   &quot;&quot;&quot; Make sure 2*2 is 4. &quot;&quot;&quot;<br>
   x=2.<br>
   result=x*x<br>
   assert approx_equal(result, 4., 1.e-6)<br>
<br>
if(__name__ == &quot;__main__&quot;):<br>
   exercise()<br>
   print &quot;OK&quot;<br>
<br>
is much easier to grasp rather than the same cluttered with the stuff<br>
(that, to add to the trouble, one needs to learn in the first place!).<br>
<br>
All the best,<br>
Pavel<br>
<br>
On 3/3/18 14:36, <a href="mailto:Graeme.Winter@diamond.ac.uk">Graeme.Winter@diamond.ac.uk</a> wrote:<br>
&gt; What’s bad about pytest?<br>
&gt;<br>
&gt;<br>
&gt;<br>
&gt;&gt; On 3 Mar 2018, at 02:26, Pavel Afonine &lt;<a href="mailto:pafonine@lbl.gov">pafonine@lbl.gov</a>&gt; wrote:<br>
&gt;&gt;<br>
&gt;&gt; Just to make sure: you are converting to use pytest this particular codes (fable), correct?<br>
&gt;&gt; Pavel<br>
&gt;&gt; P.S.: I&#39;m allergic to pytest.<br>
&gt;&gt;<br>
&gt;&gt;<br>
&gt;&gt; On 3/3/18 07:46, CCTBX commit wrote:<br>
&gt;&gt;&gt; This in preparation for pytestification.<br>
&gt;&gt; ______________________________<wbr>_________________<br>
&gt;&gt; cctbxbb mailing list<br>
&gt;&gt; <a href="mailto:cctbxbb@phenix-online.org">cctbxbb@phenix-online.org</a><br>
&gt;&gt; <a href="http://phenix-online.org/mailman/listinfo/cctbxbb" rel="noreferrer" target="_blank">http://phenix-online.org/<wbr>mailman/listinfo/cctbxbb</a><br>
&gt;<br>
<br>
______________________________<wbr>_________________<br>
cctbxbb mailing list<br>
<a href="mailto:cctbxbb@phenix-online.org">cctbxbb@phenix-online.org</a><br>
<a href="http://phenix-online.org/mailman/listinfo/cctbxbb" rel="noreferrer" target="_blank">http://phenix-online.org/<wbr>mailman/listinfo/cctbxbb</a><br>
<br>
</div></div><span class="im HOEnZb">--<br>
This e-mail and any attachments may contain confidential, copyright and or privileged material, and are for the use of the intended addressee only. If you are not the intended addressee or an authorised recipient of the addressee please notify us of receipt by returning the e-mail and do not use, copy, retain, distribute or disclose the information in or attached to the e-mail.<br>
Any opinions expressed within this e-mail are those of the individual and not necessarily of Diamond Light Source Ltd.<br>
Diamond Light Source Ltd. cannot guarantee that this e-mail or any attachments are free from viruses and we cannot accept liability for any damage which you may sustain as a result of software viruses which may be transmitted in or with the message.<br>
Diamond Light Source Limited (company no. 4375679). Registered in England and Wales with its registered office at Diamond House, Harwell Science and Innovation Campus, Didcot, Oxfordshire, OX11 0DE, United Kingdom<br>
<br>
<br>
</span><div class="HOEnZb"><div class="h5">______________________________<wbr>_________________<br>
cctbxbb mailing list<br>
<a href="mailto:cctbxbb@phenix-online.org">cctbxbb@phenix-online.org</a><br>
<a href="http://phenix-online.org/mailman/listinfo/cctbxbb" rel="noreferrer" target="_blank">http://phenix-online.org/<wbr>mailman/listinfo/cctbxbb</a><br>
</div></div></blockquote></div><br></div>