OSGeo / grass-addons

GRASS GIS Addons Repository
https://grass.osgeo.org/grass-stable/manuals/addons/
GNU General Public License v2.0
103 stars 154 forks source link

[Bug] testsuite: do not run tests for deprecated module #975

Open ninsbl opened 1 year ago

ninsbl commented 1 year ago

testsuite for addons For example, the r.in.pdal module is deprecated in addons: https://github.com/OSGeo/grass-addons/blob/grass8/src/raster/r.in.pdal/DEPRECATED

However, the testsuite for that module runs never the less and of course fails, e.g.: https://github.com/OSGeo/grass-addons/actions/runs/6706180778/job/18222154754#step:17:303

No testsuite should run for deprecated modules... Not sure how to create that exception in the testsuite, or if the test map simply should be renamed so that tests are not picked up...

neteler commented 1 year ago

I guess that things happen here:

https://github.com/OSGeo/grass-addons/blob/82d59ce1424be8d7fac485836b3af5c00bba3b33/.github/workflows/ci.yml#L91

which calls make in the respective subdirs of src/.

Here it appears that DEPRECATED modules should be ignored:

https://github.com/OSGeo/grass-addons/blob/82d59ce1424be8d7fac485836b3af5c00bba3b33/src/raster/Makefile#L5

but apparently that mechanism isn't used?

ninsbl commented 1 year ago

It seems to me, that r.in.pdal is not compiled, so the DEPRECATED file is taken into account in the make step.

When the unittest is started here: https://github.com/OSGeo/grass-addons/blob/82d59ce1424be8d7fac485836b3af5c00bba3b33/.github/workflows/test.sh#L15C16-L15C34 I guess all testsuite directory in the source tree are picked up. Maybe a solution could be to add an exception for a DEPRECATED file in the function here: https://github.com/OSGeo/grass/blob/7089dc6ef72591b807c8ab430e9b94f74f55f38b/python/grass/gunittest/loader.py#L67

wenzeslaus commented 1 year ago

Before adding more code to grass.gunittest, I suggest:

  1. Add .gunittest.cfg and exclude those tests. Deprecated code is to be removed some day which nicely fits with the line in the configure file which also should be removed eventually (other ignored files mean failing tests, so these need to be resolved too just like the line for deprecated code).
  2. Consider removing the deprecated code. You can't get that with g.extension anyway, no? If that's the case, you need to use Git to get the code. In that case, you can dig up the code from an old version using Git if you really need it.
neteler commented 10 months ago

2. Consider removing the deprecated code. You can't get that with g.extension anyway, no? If that's the case, you need to use Git to get the code. In that case, you can dig up the code from an old version using Git if you really need it.

Agreed. So:

ag -g DEPRECATED
src/raster/r.in.pdal/DEPRECATED
src/raster/r.le.patch/DEPRECATED
src/raster/r.le.pixel/DEPRECATED
src/raster/r.le.setup/DEPRECATED
src/raster/r.le.trace/DEPRECATED
src/raster/r.out.tiff/DEPRECATED

I suggest to remove all of them. Git preserves the history anyway.