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

Add GitHub Actions with build, test, and Python static code analysis #583

Merged
merged 6 commits into from
May 7, 2020

Conversation

wenzeslaus
Copy link
Member

This simply cherry picks 4bb3a3a and 39415e8 from master.

It runs Flake8, but ignores most of the errors. It is still helpful for catching some very serious ones like syntax errors.

@wenzeslaus wenzeslaus self-assigned this May 4, 2020
@wenzeslaus
Copy link
Member Author

For the tests to be at least somewhat useful without checking the result manually, this will require backport of 6962e31 (grass.gunittest.main ... --min-success).

@wenzeslaus
Copy link
Member Author

To make running tests even more useful, it would be good to include or subsequently backport also #585.

@neteler
Copy link
Member

neteler commented May 5, 2020

To make running tests even more useful, it would be good to include or subsequently backport also #585.

Since 7.8.3 is out, pls backport as needed.

…is (OSGeo#525)

This adds a CI workflow with build and test running on Ubuntu 16.04 and 18.04. The build job is not parallelized and is meant for clear and relatively fast check of compilation and building in general. This way it is clear if the compilation failed when the test job failed without examining the output. (Duplicating what is currently running on Travis.)

The test job needs to build first, but the main focus is to run tests, so the compilation is parallelized (depending on nproc) and thus potentially less readable. This runs the whole test suite. Failing tests (file names) are displayed.

The test job requires at least 50% success rate (using the --min-success flag). There is currently 45% of tests failing. Test uses nc_spm_full_v2alpha2 location from fatra.cnr.ncsu.edu for tests.

Additional work flow adds a static code analysis/code quality check/linting using Flake8 for Python code in lib/python, gui/wxpython, scripts and temporal directories (separately). As an experiment, who approaches are employed in dealing with current errors. lib/python uses a configuration file which ignores code in testsuite directories and ignores a lot of errors specified in the configuration. The other directories use the default settings and the failure is ignored using GitHub Actions configuration.

Helper files are placed to .github/workflows (possibly to be moved if general enough).
Python code errors from Flake8 are now ignored gui/wxpython, scripts, and temporal
in the same way as in lib/python. Unique set of errors is used for each directory
to keep ignored errors at minimum.

Serious errors are sorted at the top. Whitespace at the bottom.
Line length is set to 88, but ignored.
This adds numpy and other Python packages to GitHub Actions using apt.

It also increases minimal required success rate to 80% because with Python dependencies in place we are currently getting 82-84% success rate.
@wenzeslaus wenzeslaus marked this pull request as ready for review May 7, 2020 03:05
The current tests are running with 91% success rate in 7.8 branch, so let's test for that.
@ninsbl
Copy link
Member

ninsbl commented May 7, 2020

Currently builds with Ubuntu 16.04 fail while 18.04 works.
Problems are with Python dependencies. Amongst others I found the error message:
ModuleNotFoundError: No module named 'six'
E.g. line 10415 here: https://github.com/ninsbl/grass/runs/652446206

Apart from this, it is really cool to have these checks in place, so many thanks for your effort with that!

@wenzeslaus
Copy link
Member Author

Currently builds with Ubuntu 16.04 fail while 18.04 works.

Yes, the Ubuntu 16.04 job sometimes fails, emphasis on the sometimes. Look around on different branches and repos. It has never failed on the master (yet) and, for example, it runs just fine in this PR or in #596 which is from master. Nevertheless, it happened several times already (@neteler was the first one to notice). I even got it for another repo which is compiling GRASS. Re-running the job helped in my case, so you can try that just to get the green checkbox.

ModuleNotFoundError: No module named 'six'

Right, that's the error and we were not able to find anything more in the logs yet. Something bad is happening somewhere, so it would be good to get that resolved. You can see this specific problem to a lot in different repos, questions, and issues not related to GitHub Actions or GRASS GIS with both pip and apt. Reinstalling six helped according to the internet, but that seems like a strange solution for CI. From our experience, it seems it is specific to Ubuntu 16.04, but I did not follow that lead.

Anyway, this is not directly related to this PR. Please, open a new issue if needed. I'm merging this now.

...it is really cool to have these checks in place...

Thanks!

@wenzeslaus wenzeslaus merged commit 5550060 into OSGeo:releasebranch_7_8 May 7, 2020
@wenzeslaus wenzeslaus deleted the add-gh-actions-to-78 branch May 7, 2020 17:57
@wenzeslaus
Copy link
Member Author

Just for the record: There seems to be a consistent pattern of two more test files succeeding on Ubuntu 16.04 in comparison to Ubuntu 18.04.

Ubuntu 16.04:

Executed 235 test files in 0:32:45.178398.
From them 214 files (91%) were successful and 21 files (9%) failed.

Ubuntu 18.04:

Executed 235 test files in 0:38:49.221152.
From them 212 files (90%) were successful and 23 files (10%) failed.

The tests are set in 7.8 branch to consider <90% as a failure.

@mlennert
Copy link
Contributor

mlennert commented May 8, 2020

Is there actually a specific reason why we need to support Ubuntu 16.04 ?

@wenzeslaus
Copy link
Member Author

Ubuntu 16.04 is one of the two Linux VMs available in GitHub Actions and it is still an active LTS. If the errors start happening in the main repo or the noise prevents us from reviewing PRs and we still don't know how to fix it, then I would say let's remove it.

@neteler neteler added this to the 8.0.0 milestone Dec 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants