OSGeo / grass

GRASS GIS - free and open-source geospatial processing engine
https://grass.osgeo.org
Other
855 stars 310 forks source link

tests: replaced module imports to the top #4656

Closed arohanajit closed 5 days ago

arohanajit commented 2 weeks ago

Fixes E402

echoix commented 1 week ago

I'm not convinced (yet) that this is a better pattern. We don't see that often, and the real problem is the fact that plying around with the paths was used there, as our Python ctypes binding dont seem available when running plain pytest, instead of the heavy workarounds used when loading grass and executing scripts inside that shell, like our gunittest does.

In my opinion, running Python scripts like with pytest, the expected behaviour should be to just work when grass is installed. But all that is a larger infrastructure-wide issue, that @wenzeslaus has been slowly working on, and is also my common thread across all my efforts in the last year and a half.

So to recap, I think that the problem in the file is the use of the two lines that play with the path (it's really not a good practice, I saw it more in Python 2 code when users didn't understand why a script didn't work and stopped when adding enough so it works on their machine).

If it was really a test file, it could be run with the current workaround for some gunittest-tests to run through pytest: launch a grass shell, (the paths are set up), and inside call Python, pytest, or the script wanted. After reading the file finally, I see that it is not a test file finally, it is only a standalone tool. The problem with it is that it is inside a "tests" directory, that is normally used for pytest tests. It isn't picked up by pytest as it is called "benchmark", so it doesn't start or end in test (and for now pytest is configured to collect only files ending in "_test.py" inside directories named "tests"). Should it be placed somewhere else, in a graveyard with other scripts and utilities ? I see that there's a makefile that installs this, so what is it used for?

petrasovaa commented 6 days ago

I suppose this benchmark was useful when developing pygrass, less so now.