Skip to content
This repository has been archived by the owner on Oct 15, 2020. It is now read-only.

CI: enable CI for node-chakracore PRs? #427

Open
obastemur opened this issue Nov 14, 2017 · 9 comments
Open

CI: enable CI for node-chakracore PRs? #427

obastemur opened this issue Nov 14, 2017 · 9 comments

Comments

@obastemur
Copy link
Contributor

It could be nice to see what my PR is actually doing on supported systems. Especially, If there is a fail on a system that I don't currently have, it could be nice to see the logs of the fail (if any).

@digitalinfinity
Copy link
Contributor

@joaocgreis any thoughts on whether we can have GitHub CI setup for the node-chakracore repo? While we could manually trigger CI, curious if there's any reason why we can't have a web hook set up for this?

@joaocgreis
Copy link
Member

@obastemur you are a member of the node-chakracore team, which means you have run permission in https://ci.nodejs.org/view/All/job/chakracore-test-pull-request/ . Just go there, click "Build with parameters", enter the PR number and there you go. The "Certify Safe" checkbox is a reminder that the jobs do not run on sandboxes, so when running the job for code that comes from outside contributors please take a look to make sure it's not trying to run anything harmful.

@digitalinfinity about automating the whole process: in node we don't want the CI to start automatically mainly for 2 reasons: the safety issue above and the PRs frequently need adjustments before it makes sense to start CI.

@obastemur
Copy link
Contributor Author

obastemur commented Nov 14, 2017

@joaocgreis how an external contributor is able to see the fail logs? (if PR is failing on a platform)

EDIT: In other words, is there a way to do that ? previously, I wasn't able to see fail logs.

@joaocgreis
Copy link
Member

@obastemur the "Console output" is public on all jobs (logs are only kept for a few days though).

I started a run on master: https://ci.nodejs.org/job/chakracore-test/193/ and it is not going very well.. Windows failed because a node-gyp issue that is not obvious to me (I did a run last week and Windows was good: https://ci.nodejs.org/job/chakracore-test/192/). I expect Linux and OSX to fail because of processes left behind (but the console output last time showed that all tests that failed were flaky). The linter is also failing, not obvious why.

@MSLaguana
Copy link
Contributor

The windows test failure was a bug in the shim that was fixed a short time ago.

@obastemur
Copy link
Contributor Author

I see the potential issues with not enabling it by default.

However, it could be nice to get a Good to merge approval from a CI before merge. Reviewer could simply tell to CI bot to do the job..
i.e.

  • Review is done, CI test this please!

Step by step..

  • A new PR comes in.. CI receives the webhook and adds a UI box with PR is not safe to merge.. (right on top of Merge button)
  • Reviewer reviews the PR.. final checks etc..
  • Reviewer comments Review is done, CI test this please .. CI checks if reviewer is the member of the team and triggers test run.
  • CI updates the message with the result from the test run.
  • If CI message was OKAY, reviewer may merge the PR.

@joaocgreis
Copy link
Member

I'm positive there was some discussion around starting CI from GH comments, but I can't find it. The idea sounds good to me, it's possible that just no one had the time to move ahead with it yet.

cc @nodejs/github-bot

@phillipj
Copy link
Member

Yupp, the @nodejs-github-bot has functionality for triggering CI from github.com comments: nodejs/github-bot#128

Sadly it has stopped working due to some Jenkins permission issues: nodejs/github-bot#146. Might be trivial to sort out if anyone's interested in digging into the issue.

@joyeecheung
Copy link
Member

@phillipj I am interested in helping this one (as well as for other repos, mainly node core). I'll be a bit busy until Dec though

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants