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

Wed Oct 5 01:20:47 PDT 2016

[sent this from the wrong address, so it didn't get through to the list]

-------- Message d'origine --------
Envoyé : 5 octobre 2016 00:41:20 GMT+02:00

Le 4 octobre 2016 22:55:35 GMT+02:00, "Anna Petrášová" <kratochanna at gmail.com> a écrit :
>On Tue, Oct 4, 2016 at 4:22 PM, Markus Metz
><markus.metz.giswork at gmail.com> wrote:
>> On Tue, Oct 4, 2016 at 5:42 PM, Sören Gebbert
>> <soerengebbert at googlemail.com> wrote:
>>> Hi,
>>>>
>>>>
>>>> >
>>>> > You are very welcome to write the missing tests for core modules.
>>>> >
>>>> > However, i don't understand the argument that because many core
>modules
>>>> > have
>>>> > no tests, therefore new modules don't need them. If developers of
>addon
>>>> > module are serious about the attempt to make their modules usable
>and
>>>> > maintainable for others, then they have to implement tests. Its
>and
>>>> > integral
>>>> > part of the development process and GRASS has a beautiful test
>>>> > environment
>>>> > hat makes writing tests easy. Tests and documentation are part of
>coding
>>>> > and
>>>> > not something special. I don't think this is a hard requirement.
>>>> >
>>>> > There is a nice statement that is not far from the truth:
>Untested code
>>>> > is
>>>> > broken code.
>>>>
>>>> these gunittests only test if a module output stays the same. This
>>>
>>>
>>> This is simply wrong, please read the gunittest documentation.
>>
>> but then why does
>>>
>>> The gunittest for the v.stream.order addon is an example how its
>done:
>>>
>https://trac.osgeo.org/grass/browser/grass-addons/grass7/vector/v.stream.order/testsuite/test_stream_order.py
>>
>> assume certain order numbers for features 4 and 7? What if these
>order
>> numbers are wrong?
>>
>> Recently I fixed bugs in r.stream.order, related to stream length
>> calculations which are in turn used to determine stream orders. The
>> gunittest did not pick up 1) the bugs, 2) the bug fixes.
>>
>>>
>>> You can write gunittests that will test every flag, every option,
>their
>>> combination and any output of a module. I have implemented plenty of
>tests,
>>> that check for correct error handling. Writing tests is effort, but
>you have
>>> to do it anyway. Why not implementing a gunittest for every feature
>while
>>> developing the module?
>>>>
>>>>
>>>> My guess for the r.stream.* modules is at least 40 man hours of
>>>> testing to make sure they work correctly. That includes evaluation
>of
>>>> float usage, handling of NULL data, comparison of results with and
>>>> without the -m flag. Testing should be done with both high-res
>(LIDAR)
>>>> and low-res (e.g. SRTM) DEMs.
>>>
>>>
>>> Tests can be performed on artificial data that tests all aspects of
>the
>>> algorithm. Tests that show the correctness of the algorithm for
>specific
>>> small cases should be preferred. However, large data should not be
>an
>>> obstacle to write a test.
>>
>> I agree, for tests during development, not for gunittests.
>>
>> From the examples I read, gunittests expect a specific output. If the
>> expected output (obtained with an assumed correct version of the
>> module) is wrong, the gunittest is bogus. gunittests are ok to make
>> sure the output does not change, but not ok to make sure the output
>is
>> correct. Two random examples are r.stream.order and r.univar.
>
>
>I am not sure why are we discussing this, it's pretty obvious that
>gunittests can serve to a) test inputs/outputs b) catch changes in
>results (whether correct or incorrect) 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.

Well, I agree with Markus that unittests are not a panacea and that we should not fall into the trap of thinking that these tests will guarantee that the results of our modules are correct.

However, I do agree that these tests are useful in detecting if any changes to the code change the output, thus raising a flag that the developer has to at least take into account.

I'll try to write some tests for the OBIA tools when I find the time, although I do agree with Markus that it wouldn't be useful to try to write tests that would cover each and every possible corner case...

In the meantime, g.extension is wonderful tool 😃

Moritz

>
>Anna
>
>>
>> Markus M
>>
>>>
>>> Best regards
>>> Soeren
>>>
>>>>
>>>> my2c
>>>>
>>>> Markus M
>>>>
>>>> >
>>>> > Best
>>>> > Sören
>>>> >
>>>> >>
>>>> >> One thing we could think about is activating the toolbox idea a
>bit
>>>> >> more
>>>> >> and creating a specific OBIA toolbox in addons.
>>>> >>
>>>> >>> Identified candidates could be added to core once they fulfill
>the
>>>> >>> requirements above. Would that happen only in minor releases or
>would
>>>> >>> that also be possible in point releases?
>>>> >>
>>>> >>
>>>> >> Adding modules to core is not an API change, so I don't see why
>they
>>>> >> can't
>>>> >> be added at any time. But then again, having a series of new
>modules
>>>> >> can be
>>>> >> sufficient to justify a new minor release ;-)
>>>> >>
>>>> >>> Or is that already too much formality and if someone wishes to
>see an
>>>> >>> addon in core that is simply discussed on ML?
>>>> >>
>>>> >>
>>>> >> Generally, I would think that discussion on ML is the best way
>to
>>>> >> handle
>>>> >> this.
>>>> >>
>>>> >> Moritz
>>>> >>
>>>> >>
>>>> >> _______________________________________________
>>>> >> grass-dev mailing list
>>>> >> grass-dev at lists.osgeo.org
>>>> >> http://lists.osgeo.org/mailman/listinfo/grass-dev
>>>> >
>>>> > _______________________________________________
>>>> > grass-dev mailing list
>>>> > grass-dev at lists.osgeo.org
>>>> > http://lists.osgeo.org/mailman/listinfo/grass-dev
>>>
>>>
>> _______________________________________________
>> grass-dev mailing list
>> grass-dev at lists.osgeo.org
>> http://lists.osgeo.org/mailman/listinfo/grass-dev
>_______________________________________________
>grass-dev mailing list
>grass-dev at lists.osgeo.org
>http://lists.osgeo.org/mailman/listinfo/grass-dev