OSGeo4W workflow for GitHub Actions

[HuidaeCho]

Add a new workflow for compiling GRASS GIS with OSGeo4W and MSYS2 on Windows. It also runs unit tests using the grass.gunittest.main module and nc_spm_full_v2alpha2 data set.

[wenzeslaus]

@HuidaeCho The workflow in this PR runs and "succeeds", so I don't even see any errors. The OSGeo4W workflow in Checks says: OSGeo4W / windows-latest build succeeded 5 hours ago in 37s. I don't know how osgeo4w-setup-x86_64.exe and PowerShell behave. Is a non-zero exit code being ignored here? Given that almost all workflows in your repo were canceled for the latest commit on your fork, I would say there might have been some glitch. You can always re-run if you are in doubt and see then.

[HuidaeCho]

@HuidaeCho The workflow in this PR runs and "succeeds", so I don't even see any errors. The OSGeo4W workflow in Checks says: OSGeo4W / windows-latest build succeeded 5 hours ago in 37s. I don't know how osgeo4w-setup-x86_64.exe and PowerShell behave. Is a non-zero exit code being ignored here? Given that almost all workflows in your repo were canceled for the latest commit on your fork, I would say there might have been some glitch. You can always re-run if you are in doubt and see then.

That's interesting. It actually worked on this repo! I think this issue on my repo is somehow related to all those error messages I'm receiving whenever I merge upstream/master into my master. You don't have this problem on your repo? Can it be my setting? Or maybe, personal repos are different from organizational repos? Confusing. I didn't want to test my github skills in this PR, but looks like I don't have any other options now. :(

[wenzeslaus]

It actually worked on this repo! I think this issue on my repo

If it works here, you can just continue adding things and see how that develops. Did you try to re-run the jobs?

As for the success here. Is the return code actually checked? Do you see expected results?

is somehow related to all those error messages I'm receiving whenever I merge upstream/master into my master.

I don't think it should be related to how you are using Git. What error messages? The commits here and on the branch look good (both are and should be the same anyway). However, you should not be merging into your master if you do, use rebasing instead. See CONTRIBUTING.md.

You don't have this problem on your repo?

No. However, there are some problems that in forks we get that tests on Ubuntu 16.04 just take too long (>6 hours) and are killed. Never happened on master branch yet.

Can it be my setting?

I'm using the defaults, but there is not much to set anyway. You can disable yours and just see them in the PR if that really bothers you, but I would first see if there is a pattern.

Or maybe, personal repos are different from organizational repos?

If something, it would be more forks versus original repos.

[HuidaeCho]

gunittest fixes

• 32dd642: Fix path regex
• d8dd14a: Fix encoding
• 517c808: Use a non-interactive monitor for unit testing

[HuidaeCho]

test_thorough.bat times out (6 hours limit) at test_r_blend.py? Anyway, the workflow itself seems to work fine now.

@wenzeslaus I tested it on my Windows machine. test_r_blend.py opens a new monitor and it seems to wait for the monitor to be closed (?). The moment I closed the monitor, the test moved to the next one. Changed wx0 to the non-interactive png monitor in 517c808. It will create testreport/scripts/r.blend/test_r_blend/map.png.

[wenzeslaus]

This is awesome!

The test in r.blend is not well written to start with, but you fix makes it not harmful, so I second that. The encoding seems seems to be working and it makes sense to me, so I would go with that. The YAML file and the scripts seem to be split reasonably. I'm adding some minor comments here and there.

One thing to double check is that it really fails if the tests fail. I think you confirmed it fails at one point, but I said, double checking, because I don't think we will be examining the textual output for each PR/commit especially once we increase our success rate there in the future.

However, I myself don't compile on Windows, so it would be good if someone else who compiles GRASS GIS on Windows adds a second review. (@landam @hellik ?)

[HuidaeCho]

One thing to double check is that it really fails if the tests fail. I think you confirmed it fails at one point, but I said, double checking, because I don't think we will be examining the textual output for each PR/commit especially once we increase our success rate there in the future.

Assuming that each and every unit test script is written well and reports correct results to grass79.py, I confirmed that the workflow actually fails when the success rate drops below --min-success.

I'm just wondering why around 20% tests are still failing on Linux builds. 40% failure on OSGeo4W is really poor, but it's understandable (?)... Are we 100% confident that all unit tests are written well?

[wenzeslaus]

Assuming that each and every unit test script is written well and reports correct results to grass79.py, I confirmed that the workflow actually fails when the success rate drops below --min-success.

Yes, grass...--exec passes the non-zero return code of whatever is being executed and gunittest now newly return non-zero when at least one test fails. The tests failing or not that's of course another story, but here we can just assume it works.

I'm just wondering why around 20% tests are still failing on Linux builds. 40% failure on OSGeo4W is really poor, but it's understandable (?)...

Ideally, we would have 100% tests passing and it is both realistic and reasonable requirement. The tests running on my Linux server were above 90% if everything was all right (2014-08-01 till 2019-03-26):

http://fatra.cnr.ncsu.edu/grassgistests/summary_report/nc/index.html

Unlike the Python code checks, I did not review the tests in detail after adding them to GitHub Actions, so there might be some trivial problems. From running them on my machine, I can see from the reports that there is a group of tests failing in relation to band references in temporal framework. The errors don't show in the GitHub Actions, but that's fixable.

Are we 100% confident that all unit tests are written well?

As you saw with the r.blend test, no, but that one is also extreme case which slipped review during Google Code-in. Some tests are from the pre-test-automation era, so they may or may not work and are waiting to be corrected. Other than that, it is really case by case and examining the failing test is necessary.

[wenzeslaus]

@hellik @landam It would be great to have at least one additional review for this (see the note in my review).

[hellik]

are the binaries downloadable somewhere? could these be used to upload for daily builds in OSGeo4W?

[HuidaeCho]

Not at this point, but we could use GitHub artifacts to copy out the package: https://help.github.com/en/actions/configuring-and-managing-workflows/persisting-workflow-data-using-artifacts

[hellik]

ok.

if we're going this way later on, maybe the install scheme (now in \opt) have to be adapted to be usable in OSGeo4W?

[HuidaeCho]

The current workflow is mainly for testing, but changing the install scheme wouldn't be a problem. I thought about that possibility too, but we already have working daily builds for Windows. Also, I wanted to create a portable package that doesn't require admin rights to install.

[hellik]

@wenzeslaus :had a quick review regarding windows compilation, looks good so far. if we're using it later on with GH artifacts (and OSGeo4W daily builds), maybe some fine tuning is needed.

[wenzeslaus]

Thanks @hellik for review. We won't be waiting for @landam and merge.

@HuidaeCho can you please change the PR description to reflect what have you done so we can use it as a commit message?

[HuidaeCho]

PR for artifact archiving in #1212.