Graal: Require Travis CI to pass before merging PRs

Created on 24 Jun 2020  路  15Comments  路  Source: oracle/graal

Feature request

The travis CI pipeline of graalvm has been failing for quiet some time.
Since this is the only CI that community contributors have access to, I suggest implementing a policy that requires the travis CI pipeline to pass before merging any PR.

Is your feature request related to a problem? Please describe.
The graal repo looks "broken" at a first glance since the only testing it comes with is failing.
Additionally, it makes it hard for contributors to see whether they introduce any breaking changes.

Describe the solution you'd like.
Github supports enabling a policy that requires all tests to pass before allowing a merge. I suggest enabling this policy or a similar one.

Describe who do you think will benefit the most.
GraalVM contributors.

Express whether you'd like to help contributing this feature
I could assist in getting the tests to pass.

feature

All 15 comments

@zakkak I agree something like this should be in place. Currently, however, the GraalVM team reviews PRs using their internal tooling and run CI on their infrastructure to ensure that nothing breaks on their end (some parts of the code base are not public, e.g. EE code and Oracle DB integration). So PRs are currently not directly merged on GitHub and therefore bypass GitHub checks. We've talked about this at the community workshop and it came up in the Advisory Board meetings. The SVM team and others are using GitHub issue more and more rather than doing everything in their internal tracking system. Let's hope something similar will happen with the CI infrastructure as well at some point.

Indeed, it's not easy to incorporate the Travis gate into the Oracle internal gate.
I've looked through the current failures and they break down into these 2 categories:

  • Eclipse formatting failures. I am working to get the one line fix for this merged soon.
  • It seems like python-virtualenv is no longer available on the ARM64 Travis machines. I assumed this used to work. @zakkak it would be great if you could investigate that further.

In lieu of integrating the Travis gate into the internal gate, we could treat the Travis gate in a similar way to our post-merge internal weekly tests. When these tests fail, an email is sent to a dedicated "gate keeper" who triages the failure(s) further. It would be great to have someone from the GraalVM community volunteer for the gate keeper role for the Travis gate.

What about the point where the Oracle repo is mirrored out into GitHub?

You could modify that process to instead open a branch, let Travis run, check the result of Travis, and then merge the branch if it passes. If it doesn't pass, then don't merge, and notify someone.

(I suggest you also move off Travis - it's clearly a sinking ship at this point.)

Interesting idea but not quite as easy to set up. I'd prefer to try the gate keeper approach until it proves unworkable.

Another option would be to make the CI based completely on Docker images, run on something like GitHub Actions, then anyone can run locally. Then Oracle can run the public CI, internally as well.

+1 on github actions driven builds instead for community.

I don't think GitHub Actions offers AArch64 GitHub-hosted runners yet so we'd need to maintain at least an AArch64 Travis based gate.

I opened a different issue for the travis -> github discussion (https://github.com/oracle/graal/issues/2609)

@dougxc I will have a look at the python-virtualenv issue. Thanks

I don't think GitHub Actions offers AArch64 GitHub-hosted runners yet so we'd need to maintain at least an AArch64 Travis based gate.

there are options like https://github.com/uraimo/run-on-arch-action or hosting own runner.

I gave run-on-arch-action a go today and managed to build and run TruffleSqueak on emulated aarch64:
https://github.com/hpi-swa/trufflesqueak/runs/808862521?check_suite_focus=true#step:3:1829

@dougxc I guess now that the slack notification is working we don't need a designated gate keeper, do we?

Regarding PRs on github the policy can be easily enforced by requiring all tests to pass before merging (not sure how easy/hard it is to integrate this in the graalvm-bot though).
Regarding the internal merges (which at some point would be nice to go through public PRs as @chrisseaton suggests) I guess slack notifications are OK since I expect breakages to be due to some incompatibility between internal CI and travis configuration (like the eclipse version, etc.).
WDYT?

No need for a designated gate keeper now since everyone subscribed to #travis-ci can now share that responsibility.

As you hint, enforcing the Travis gate to pass when we mirror out to GitHub would require significant re-work of the mirroring process. For now, we will rely on the Slack notifications to detect and fix Travis gate issues.

Thanks @dougxc

Feel free to close this issue, or leave it open till the gating process gets automated.

I will close for now and re-open if we decide to integrate the Travis gate in the mirroring or internal gating process.

Was this page helpful?
0 / 5 - 0 ratings