Wednesday, 8 November 2017

Test Etiquette

Fig 1: A brigade of woobles, apparently
Today, we're going to talk about testfest, in case you have no idea what that is, here is an excerpt from the website:
Have you ever wanted to contribute to PHP but have been afraid that your C skills aren’t up for the challenge? Well, have no fear! If you know PHP, you can contribute by writing tests. Through your local user group, PHP TestFest will show you how.
I've long been trying to propogate the view that you don't need to be a master of anything (including C) to be a valuable contributor to PHP. While the language is written in C, absolutely everything else - tests, websites, admin systems, documentation - is written in languages we all understand; PHP, XML, and Javascript.

Test Fest is focused on turning you into a contributor to php-src by educating you on how to write tests for PHP, but there is lots for everyone to do.

The idea is that you attend local meetups and are guided in the practicalities of writing tests for PHP, with the goal to improve the quality of the test suite that is part of continuous integration and quality control.

I've reviewed nearly all of the pull requests made so far, and approved the vast majority of them. Some of them I'm not able to approve, for a few reasons. First, I want to look at those reasons in the hope that user group organizers and participants will see this information and so reduce the amount of rejection.



Test Coverage

Coverage shouldn't be the primary driver for adding new tests, quality should.

Nevertheless, coverage is a good indicator that you are not duplicating tests - a thing we obviously want to avoid.

There exists a web interface for viewing coverage information for PHP tests at gcov.php.net.

For anyone organizing a meetup, gcov needs to be front and centre: With such a vast number of tests, and already a reasonably high level of duplication we must make sure that coverage is not further duplicated.

For anyone writing tests: It would be helpful, although not necessary, if your pull request linked to gcov (you can only link to whole files, not individual lines) and included the details of the lines (or functions) you are trying to cover. This makes review that much easier, and so probably quicker.

Testing ZPP

ZPP - zend_parse_parameters, is in some form called upon entry to nearly every internal function in PHP. It accepts the number of arguments that were passed, and a specification string describing what the function expects to find on the argument stack. By convention, if the stack does not contain expected arguments, the function or method should return immediately leaving the return value set null (or undefined).

Because this is such a widely used function, it has been tested roughly one squillion times over, and there's not much sense in testing convention.

For anyone organizing a meetup: Please make clear that we don't want to test ZPP.

For anyone writing tests: If you are writing a test and are EXPECT'ing something that matches:
expects parameter %d to be %s, %s given
Then you are testing ZPP, unnecessarily, and while coverage may report an increase, it's improved quality we are looking for.

Clear Intentions

This may seem more subjective, harder to get right, but it's not different to writing normal code: The intention behind any code should be clear. Just as if you find yourself writing long comments in normal code, it may need to be refactored; If you find yourself writing complex and cryptic descriptions probably the test needs to be refactored.



Lastly I want to encourage anyone and everyone to find a local meetup that is participating in testfest and attend, even if it's your first time: All of us depend on the tests that PHP is distributed with, and all of us are qualified to improve the quality of those tests.

Imagery is © 2017 PHP Community Foundation.

No comments:

Post a Comment