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] Many tests fail with issues related to band references #1342

Closed
wenzeslaus opened this issue Feb 15, 2021 · 18 comments · Fixed by #1447
Closed

[Bug] Many tests fail with issues related to band references #1342

wenzeslaus opened this issue Feb 15, 2021 · 18 comments · Fixed by #1447
Assignees
Labels
bug Something isn't working temporal Related to temporal data processing
Milestone

Comments

@wenzeslaus
Copy link
Member

Describe the bug
Number of tests fail with band references related issues.

./temporal/t.snap/testsuite/test_snap.py:
sqlite3.OperationalError: no such column: number_of_bands
./temporal/t.info/testsuite/test.t.info.sh:
sqlite3.OperationalError: table str3ds_metadata has no column named number_of_bands
...

To Reproduce
See any test run report, e.g.,

  1. Go to https://github.com/OSGeo/grass
  2. Click on symbol for CI runs/checks (checkmark, x, dot)
  3. Find CI / ubuntu-20.04 tests (or 18.04)
  4. Click Details
  5. Find Artifacts somewhere in the top right area
  6. Click testreport-ubuntu-20.04
  7. After download, unzip
  8. Open index.html in your web browser

Expected behavior
Tests not failing or at least failing with the same errors as before introduction of band references.

@wenzeslaus wenzeslaus added bug Something isn't working temporal Related to temporal data processing labels Feb 15, 2021
@neteler
Copy link
Member

neteler commented Feb 15, 2021

At time the dataset http://fatra.cnr.ncsu.edu/data/nc_spm_full_v2alpha2.tar.gz (which has been created for GRASS GIS < 7.9) is used as-is also in GRASS GIS 7.9.

To run TGRASS related tests successfully, first t.upgrade (addon) must be run in the relevant mapsets (PERMANENT etc) to update the TGIS schema to include the band references.

I prepare fixes for the tests of the modules g.bands and i.band separately.

@wenzeslaus
Copy link
Member Author

Can't we really have this backward compatible? Many users will have this issue. Just opening a GUI for a temporal module triggers the database creation.

At time the dataset http://fatra.cnr.ncsu.edu/data/nc_spm_full_v2alpha2.tar.gz (which has been created for GRASS GIS < 7.9) is used as-is also in GRASS GIS 7.9.

We need an official version of this dataset, but that's a different story.

To run TGRASS related tests successfully, first t.upgrade (addon) must be run in the relevant mapsets (PERMANENT etc) to update the TGIS schema to include the band references.

Why an addon? v.build.all is not an addon, but then again, is this as serious as the vector topology change?

@neteler
Copy link
Member

neteler commented Feb 15, 2021

Can't we really have this backward compatible?

Not sure as the DB schemas are different (hence the version number). Ideas are certainly welcome.

Many users will have this issue. Just opening a GUI for a temporal module triggers the database creation.

...

To run TGRASS related tests successfully, first t.upgrade (addon) must be run in the relevant mapsets (PERMANENT etc) to update the TGIS schema to include the band references.

Why an addon? v.build.all is not an addon, but then again, is this as serious as the vector topology change?

(AFAIR) Nobody requested so far to move it to core. The idea was proposed in #130 and implemented in OSGeo/grass-addons#71
I am always confused to not have it in G79 itself.

@wenzeslaus
Copy link
Member Author

At time the dataset http://fatra.cnr.ncsu.edu/data/nc_spm_full_v2alpha2.tar.gz (which has been created for GRASS GIS < 7.9) is used as-is also in GRASS GIS 7.9.

Same errors when you run tests on newly downloaded standard nc_spm_08_grass7 which does not have any tgis/sqlite.db.

To run TGRASS related tests successfully, first t.upgrade (addon) must be run in the relevant mapsets (PERMANENT etc) to update the TGIS schema to include the band references.

nc_spm_full_v2alpha2 has tgis/sqlite.db only in PERMANENT. The automated tests are running in new separate mapsets.

@veroandreo
Copy link
Contributor

Describe the bug
Number of tests fail with band references related issues.

./temporal/t.snap/testsuite/test_snap.py:
sqlite3.OperationalError: no such column: number_of_bands
./temporal/t.info/testsuite/test.t.info.sh:
sqlite3.OperationalError: table str3ds_metadata has no column named number_of_bands
...

AFAIU, from the 4 temporal tests failing today, only 2 tests fail with the above error and both of them imply the creation of a STR3DS, i.e., they use 3D raster data. Does the concept of bands apply also there or was included also for STR3DS?? In my opinion, that's at least arguable.

@neteler
Copy link
Member

neteler commented Mar 10, 2021

Same errors when you run tests on newly downloaded standard nc_spm_08_grass7 which does not have any tgis/sqlite.db.

Please provide a reproducible example which I am happy to try (due to lack of time it is not obvious to me to define this test myself).

@wenzeslaus
Copy link
Member Author

Same errors when you run tests on newly downloaded standard nc_spm_08_grass7 which does not have any tgis/sqlite.db.

Please provide a reproducible example which I am happy to try (due to lack of time it is not obvious to me to define this test myself).

grass --tmp-location XY --exec g.extension g.download.location
mkdir /tmp/grassdata
grass --tmp-location XY --exec g.download.location dbase=/tmp/grassdata url=https://grass.osgeo.org/sampledata/north_carolina/nc_spm_08_grass7.zip
cd grass/gis/source/code
grass --tmp-location XY --exec python3 -m grass.gunittest.main --grassdata /tmp/grassdata/ --location nc_spm_08_grass7 --location-type nc --min-success 80

@neteler
Copy link
Member

neteler commented Mar 10, 2021

I get this error, not sure if related:

grass79 --tmp-location XY --exec python3 -m grass.gunittest.main --grassdata /tmp/grassdata/ --location nc_spm_08_grass7 --location-type nc --min-success 80
Starting GRASS GIS...
Creating new GRASS GIS location <tmploc>...
Cleaning up temporary files...
Executing <python3 -m grass.gunittest.main --grassdata /tmp/grassdata/ --location nc_spm_08_grass7 --location-type nc --min-success 80> ...

Executed 0 test files in 0:00:00.009054.
From them 0 files (unknown percentage) were successful and 0 files (unknown percentage) failed.
Traceback (most recent call last):
  File "/usr/lib64/python3.9/runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/lib64/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/home/mneteler/software/grass_master/dist.x86_64-pc-linux-gnu/etc/python/grass/gunittest/main.py", line 227, in <module>
    sys.exit(main())
  File "/home/mneteler/software/grass_master/dist.x86_64-pc-linux-gnu/etc/python/grass/gunittest/main.py", line 221, in main
    if reporter.file_pass_per >= args.min_success:
TypeError: '>=' not supported between instances of 'NoneType' and 'int'

python --version
Python 3.9.2

@wenzeslaus
Copy link
Member Author

Executed 0 test files in 0:00:00.009054.

Perhaps a missing cd to source code?

@wenzeslaus
Copy link
Member Author

TypeError: '>=' not supported between instances of 'NoneType' and 'int'

I created #1445 (a "good first issue") for that traceback, so hopefully there will be less confusion in the future.

@landam landam self-assigned this Mar 11, 2021
@landam landam added this to the 8.0.0 milestone Mar 11, 2021
@landam landam linked a pull request Mar 11, 2021 that will close this issue
@landam
Copy link
Member

landam commented Mar 11, 2021

After applying #1447 the test

./temporal/t.snap/testsuite/test_snap.py:
sqlite3.OperationalError: no such column: number_of_bands

is passing

./bin.x86_64-pc-linux-gnu/grass79 -c /tmp/grassdata/nc_spm_08_grass7/test_t_snap01 --exec python3 ./temporal/t.snap/testsuite/test_snap.py
...
Ran 6 tests in 16.554s
OK

@landam
Copy link
Member

landam commented Mar 11, 2021

Also artifacts seems to be better

Screenshot_2021-03-11 Screenshot

@wenzeslaus
Copy link
Member Author

Great, here is the relevant porting of the output taken from the raw log of #1447 for both t.info and t.snap from the original description:

2021-03-11T17:28:22.4927193Z Running test_distr_tgis_db_vector (./temporal/t.connect)...
2021-03-11T17:28:41.9967902Z Running test.t.info (./temporal/t.info)...
2021-03-11T17:29:11.1905672Z Running test.t.merge (./temporal/t.merge)...
...
2021-03-11T17:42:01.0926894Z Running test_shift (./temporal/t.shift)...
2021-03-11T17:42:34.6110018Z Running test_snap (./temporal/t.snap)...
2021-03-11T17:42:55.5243717Z Running test_support_str3ds (./temporal/t.support)...

@wenzeslaus
Copy link
Member Author

wenzeslaus commented Mar 12, 2021

For future reference and to clarify that this can be closed soon, it might be worth summarizing this issue and what the discussion here touched:

@wenzeslaus
Copy link
Member Author

...from the 4 temporal tests failing today, only 2 tests fail with the above error...

@veroandreo Any idea how to get to these two numbers, e.g., with the current master branch? I checked the logs at the time and though I see the same, so I concluded the other failing ones I saw when I reported the issue must have been the other issues. However, now I compared master branch and #1447 results and I get 11 tests fixed by it. If you don't know, let's just leave this unresolved, but if you have some idea, I would like to clarify this to be sure there is nothing wrong with the testing (like, for example, some tests being skipped sometimes).

@veroandreo
Copy link
Contributor

...from the 4 temporal tests failing today, only 2 tests fail with the above error...

@veroandreo Any idea how to get to these two numbers, e.g., with the current master branch? I checked the logs at the time and though I see the same, so I concluded the other failing ones I saw when I reported the issue must have been the other issues. However, now I compared master branch and #1447 results and I get 11 tests fixed by it. If you don't know, let's just leave this unresolved, but if you have some idea, I would like to clarify this to be sure there is nothing wrong with the testing (like, for example, some tests being skipped sometimes).

Honestly, I do not know what could be wrong there. I cannot access #1447 results now. Can I re-run them?

I guess it does not make sense to see other PRs, no? But, checking the results of #1459 that are still available, I do see many tests failing with the number_of_bands error as described in this issue.

@wenzeslaus
Copy link
Member Author

I think I found the reason why we saw just two failing tests.

Honestly, I do not know what could be wrong there. I cannot access #1447 results now. Can I re-run them?

They are still there, but there everything was as expected.

I guess it does not make sense to see other PRs, no?

Other PRs or master branch, it would not matter for finding the state without #1447.

But, checking the results of #1459 that are still available, I do see many tests failing with the number_of_bands error as described in this issue.

Right. I see the same. What I think happened is that both of us checked the per-step output in job view in GitHub Actions. Nothing wrong with that, I use it all the time, but what just happened to me now and what I think was the case before, is that I just opened the step (> -> v) and checked first few lines, then used Ctrl+F in the web browser and found just two. Not all text is loaded (you would see them if you would scroll through). When I opened the raw log (⋯ -> View raw logs), searching revealed much more.

Conclusion, everything works as expected, but things like counting or searching are better done in the raw log or the artifact with results.

@landam
Copy link
Member

landam commented Mar 18, 2021

Can we summarize what is missing to close this issue? Ideally

  • missing1
  • missing2

Thanks.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working temporal Related to temporal data processing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants