[GRASS-dev] Fwd: Re: Upcoming 7.2.0: review which addons to move to core

[Blumentrath, Stefan]

Hi again,

Interesting discussion!

With my user and amateur addon-developer perspective I would conclude that:
- we all agree that tests/unittests are very useful even if not "the silver bullet"
- things one does in "unittests" might also partly be done in the code of the module (e.g. check for a specific state of input or environment variables ...), so a requirement for unittest might have to go hand in hand with a requirement for "maturity" of the module?
- "unittests" can also differ a lot in complexity so it is probably not too easy to say which level of unit tests should be required for a new core module, and the presence of a test alone does not necessarily mean that a module is "well tested"...
- it might be a good starting point to "require" only "simple" unittests for new core modules in that sense that a unit test makes sure that at least examples from the manual work without throwing an error? That would at the same time test code and manual/documentation... There was the idea to use examples from the manual to auto-generate tests, was`nt it?
- with more tests on "higher level" modules, also "lower level functions" (e.g. library) might be covered. This could on the one hand help to trace (side) effect of changes on lower level functions, but on the other hand might require to also update all output-tests which were written based on that bogus library function, would not it?
- like some others here I never wrote a test for my addons, but I would be willing to learn! Here the simple examples Vaclav provided were very helpful, as for me as a rookie the more involved examples and the full documentation of the unittests are hard to understand. Maybe you can add a very minimalistic example to the test documentation that only checks (e.g. based on NC data set) that e.g. "r.example -g input=mymap" works without throwing an error?

And finally, Martins request regarding an RFC for "promoting modules to core" should not drown in the discussion about unittests (even if the latter probably is a precondition to the former)...

Cheers
Stefan

P.S.: For automatized tests of addons there might be a dependency issue, as a test-server would have to be equipped with all possible dependencies in order to be able to run the addons?



-----Original Message-----
From: grass-dev [mailto:grass-dev-bounces at lists.osgeo.org] On Behalf Of Moritz Lennert
Sent: 7. oktober 2016 08:32
To: Vaclav Petras <wenzeslaus at gmail.com>; Anna Petrášová <kratochanna at gmail.com>
Cc: GRASS developers list <grass-dev at lists.osgeo.org>; Markus Metz <markus.metz.giswork at gmail.com>
Subject: Re: [GRASS-dev] Fwd: Re: Upcoming 7.2.0: review which addons to move to core

On 06/10/16 23:34, Vaclav Petras wrote:
> On Tue, Oct 4, 2016 at 4:55 PM, Anna Petrášová <kratochanna at gmail.com 
> <mailto:kratochanna at gmail.com>> wrote:
>
>     [...]
>
>     c) test correctness of results.
>     It just depends how you write them, and yes, for some modules c) is
>     more difficult to implement than for others.
>
>
> On Thu, Oct 6, 2016 at 5:00 PM, Anna Petrášová <kratochanna at gmail.com 
> <mailto:kratochanna at gmail.com>> wrote:
>
>     There is correct result in the sense "as author intended". So you can
>     use pen and paper and solve very small example. That doesn't mean it
>     will cover all options, but better than nothing. [...]
>
>
> For r.forestfrag, I wrote a test which was based on an example in the 
> original paper which was computing value of one cell in 3x3 window. It 
> is a really trivial example which tests 1 number and two other numbers 
> which are intermediate results. However, by writing it, I actually 
> discovered that although the results for a large area look good, this 
> small example, which I was able compute by hand, is failing. After 
> investigation, it turned out that the error is actually in r.mapcalc.
> Computing the result outside of GRASS was crucial in this case. It was 
> an index and I was able to compute it by hand (and check it with the 
> example in the paper). For some other modules it could be more 
> difficult and I don't want to compute it without GRASS for more than 
> one cell even in this case, but it was certainly possible to write a 
> test of correctness for this module. Note that the test doesn't show 
> if the (forest fragmentation) index makes sense or if it is useful, 
> but it shows that the module gives the result it promised.
>
> This is the original test:
>
> https://trac.osgeo.org/grass/browser/grass-addons/grass7/raster/r.fore
> stfrag/testsuite/r_forestfrag_trivial.py

Nice example, thanks !

>
> This is the r.mapcalc bug:
>
> https://trac.osgeo.org/grass/ticket/3067
>
> And this is test which specifically shows the bug (would show if it 
> would be reintroduced):
>
> https://trac.osgeo.org/grass/browser/grass/trunk/raster/r.mapcalc/test
> suite/test_row_above_below_bug.py

Here we have an example of a test with 70 lines of code which was comitted almost at the same time as the issue was fixed (ok 3 days before), with a one-line fix. Now, it's good to safeguard against this bug reappearing, but I would argue that this example shows that we often only think of relevant tests after a bug appears, and I'm not sure that it is very efficient to write a test at that point, given the low probability of that bug reappearing ;-)

But hey, bear with me, I'm learning...

Moritz
_______________________________________________
grass-dev mailing list
grass-dev at lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev