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

[Markus Metz]

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.

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
>
>