Add GH Actions workflow for integration testing with Sage

sagemath / cysignals

cysignals: interrupt and signal handling for Cython. Source repository for https://pypi.org/project/cysignals/

GNU Lesser General Public License v3.0

44 stars 23 forks source link

Add GH Actions workflow for integration testing with Sage #139

Closed mkoeppe closed 2 years ago

mkoeppe commented 3 years ago

This can be seen in action at https://github.com/mkoeppe/cysignals/runs/2069485461?check_suite_focus=true

embray commented 3 years ago

This is quite a tour de force. Have you used this for checking other packages' Sage integration?

embray commented 3 years ago

This runs so many jobs, and a lot of them are failing. How do I know which failures matter and which ones don't (or do they?)

embray commented 3 years ago

I haven't really kept up with what you've been doing with tox, but it's pretty mind-bending. I never would have even thought of using tox to run tests in Docker, much less have it do things like install Cygwin packages. Does tox even create Python virtualenvs in these cases? It seems like there would be no reason for it to but I'm still pretty confused about how this works.

mkoeppe commented 3 years ago

This is quite a tour de force. Have you used this for checking other packages' Sage integration?

Yes, a version of this has been in use for a while now with Normaliz, arb, e-antic; and most recently a PR adding it to Singular was accepted.

mkoeppe commented 3 years ago

Does tox even create Python virtualenvs in these cases? It seems like there would be no reason for it to

It does, but the venvs are basically empty.

mkoeppe commented 3 years ago

This runs so many jobs,

Currently half of them are for macOS, where we are testing an excessive amount of variants because of the challenging Big Sur situation.

and a lot of them are failing. How do I know which failures matter and which ones don't (or do they?)

This is a known weakness of this approach of sending Sage's portability infrastructure to upstream packages: Unrelated issues in dependencies of the tested package and instabilities in the distributions can lead to failures, which may be hard to understand, or just annoying, for the maintainer of the upstream package.

The failure for cygwin-minimal looks real to me:

src/cysignals/tests.pyx
killed by signal: 11
make[3]: *** [Makefile:71: check-doctest] Error 1

Also ubuntu-trusty-minimal looks like a real error in cysignals, but I don't see an error message.

I haven't looked at the other failing runs

mkoeppe commented 3 years ago

ping?

embray commented 3 years ago

I wonder if we could reduce the number of variants to ones that just already work, just for starters (so I'm merging something that's working into master), than open a separate PR (or PRs) for adding other variants once they're working.

mkoeppe commented 3 years ago

That's fine with me, please go ahead - certainly green checkmarks are more reassuring to look at

mkoeppe commented 3 years ago

Do you want this on master or on 1.10.x?

mkoeppe commented 2 years ago

@kliem Running at https://github.com/mkoeppe/cysignals/actions/runs/1283405525

kliem commented 2 years ago

This seems to work and I would propose on merging this, so that we have something to test pull requests with.

mkoeppe commented 2 years ago

Sounds good to me

kliem commented 2 years ago

So for merging we comment out lines 62--64 and leave them there so we can use it, once we need it.

mkoeppe commented 2 years ago

So for merging we comment out lines 62--64 and leave them there so we can use it, once we need it.

I think it also works as is. When a change in Sage is needed, it will suffice to just put it on the branch of the target ticket https://trac.sagemath.org/ticket/32579

kliem commented 2 years ago

I see.

kliem commented 2 years ago

This workflow has already proven itself very useful at #151 (edited to the correct number). Thank you.