Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
ninsbl opened this issue Oct 31, 2023 · 4 comments
Open

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

ninsbl opened this issue Oct 31, 2023 · 4 comments
Labels
CI Continuous integration Python Related code is in Python

Comments

@ninsbl
Copy link
Member

ninsbl commented Oct 31, 2023

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

@ninsbl ninsbl added CI Continuous integration Python Related code is in Python labels Oct 31, 2023
@neteler
Copy link
Member

neteler commented Oct 31, 2023

I guess that things happen here:

cd grass-addons/src

which calls make in the respective subdirs of src/.

Here it appears that DEPRECATED modules should be ignored:

SUBDIRS := $(filter-out $(DEPRECATED_SUBDIRS), $(ALL_SUBDIRS))

but apparently that mechanism isn't used?

@ninsbl
Copy link
Member Author

ninsbl commented Oct 31, 2023

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
Copy link
Member

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
Copy link
Member

neteler commented Jan 4, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous integration Python Related code is in Python
Projects
None yet
Development

No branches or pull requests

3 participants