[OpenLayers-Dev] Patches Need Tests

[John Cole]

As someone who lives by unit tests, I wholeheartedly endorse this.  I
require my developers to include unit tests before passing a code review
(otherwise how does the reviewer know the patch works).  While I haven't had
the time just yet to delve into OpenLayers code just yet, instructions for
performing testing on JS apps like OL would be fantastic, and something that
I would take advantage of when coding JS using OL.

And when a unit test won't work, functional examples should be included and
the instructions for doing so should explain how to create these as well.
Since it seems the team has done both in the past, hopefully it won't be too
difficult to create the documentation for this.

John

-----Original Message-----
From: dev-bounces at openlayers.org [mailto:dev-bounces at openlayers.org] On
Behalf Of Cameron Shorter
Sent: Saturday, February 17, 2007 2:08 PM
To: Christopher Schmidt
Cc: dev at openlayers.org
Subject: Re: [OpenLayers-Dev] Patches Need Tests

Chris,
I think that your guidelines should specify that if someone provides a 
patch, then they should also provide a patch for the tests for that 
file. It would help if there is a "Writing Unit Tests" at 
http://trac.openlayers.org/wiki/CodingStandards has some more detail:
* Note the JS tool being used for testing.
* Point to the tool's documentation explaining how to write tests
* Probably write a short howto explaining what is expected from users.
 I'd expect to see writing Unit Tests added as a requirement to "Upload 
your patch" linked from http://trac.openlayers.org/wiki/HowToContribute

Unit testing in OpenLayers helps ensures a level of quality which 
OpenLayers has become known for. I think it is something that is 
relatively easy to sell to contributors.
(I realize my track record with writing OL UTs is poor, but doesn't stop 
me believing they should be mandated).

Do you testing tools provide you with a code coverage report?

Christopher Schmidt wrote:
> Recently, we've been getting, and applying a large number of patches
> from the community. These patches have helped to solve dozens of bugs
> that might have otherwise gone undiscovered in the OpenLayers code base. 
>
> However, our testing of these changes has fallen by the wayside. In the
> past, I took the time out to write tests for most of the patches that I
> committed -- anything that could be tested programatically. Recently, I
> haven't taken this time out: patches have been committed which have no
> tests, and this is the kind of thing that leads to regressions. 
>
> I'd like to see more patches accompanied by tests. Understandably, not
> all problems can be easily tested in an automated way. An example of
> this is the init tiles issue, #480. I was unable to come up with an easy
> way to write an automated test for this, so instead, I wrote a simple
> HTML page which demonstrated the problem, with a description of what
> should be seen and what shouldn't:
>
>   http://openlayers.org/dev/tests/grid_inittiles.html
>
> I understand that there are cases where this is the case, and I would be
> much happier accepting a patch if it had even a non-automated test that
> described the problem it was solving, and how to reproduce it, and what
> it should look like if it has passed or failed. 
>
> I'm speaking solely for myself here, but I'm leaning towards pushing
> back on patch writers to write some kind of tests that go along with
> patches where possible. An example of this is ticket #496. In that case,
> I was able to write tests:
>
>   http://trac.openlayers.org/changeset/2237
>
> In the future, I'd like to see patch authors include these tests where
> possible.
>
> Is there anything that can be done to make the test-writing process
> simpler? Do we need more docs on the test writing stuff, or is this just
> something people hadn't been thinking about?
>
> Regards,
>   


-- 
Cameron Shorter
Systems Architect, http://lisasoft.com.au
Tel: +61 (0)2 8570 5011
Mob: +61 (0)419 142 254

_______________________________________________
Dev mailing list
Dev at openlayers.org
http://openlayers.org/mailman/listinfo/dev

-- 
No virus found in this incoming message.
Checked by AVG Free Edition.
Version: 7.5.441 / Virus Database: 268.18.1/690 - Release Date: 2/16/2007
2:25 PM
 

-- 
No virus found in this outgoing message.
Checked by AVG Free Edition.
Version: 7.5.441 / Virus Database: 268.18.2/692 - Release Date: 2/18/2007
4:35 PM
 
This email and any files transmitted with it are confidential and intended solely for the use of the individual or entity to whom they are addressed. If you have received this email in error please notify the sender. This message contains confidential information and is intended only for the individual named. If you are not the named addressee you should not disseminate, distribute or copy this e-mail.