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

A job for testing node-addon-api #844

Closed
gabrielschulhof opened this issue Aug 22, 2017 · 17 comments
Closed

A job for testing node-addon-api #844

gabrielschulhof opened this issue Aug 22, 2017 · 17 comments

Comments

@gabrielschulhof
Copy link

We need a job for testing a commit in the node-addon-api repo. We're running into problems which could have been prevented with a CI:

nodejs/node-addon-api#116
nodejs/node-addon-api#115
nodejs/node-addon-api#114

The job would not build Node.js at all, but would use nvm or nvs to grab the official release and test node-addon-napi against it.

@gabrielschulhof
Copy link
Author

@mcollina, right?

@refack
Copy link
Contributor

refack commented Aug 22, 2017

LGTM
(I would like to take a shot at implementing)

@mcollina
Copy link
Member

@gabrielschulhof yes.

Thanks @refack!

@gibfahn
Copy link
Member

gibfahn commented Aug 22, 2017

I can help get you guys set up (creating Jenkin jobs etc), and then @refack can help with configuration stuff.

@gibfahn gibfahn self-assigned this Aug 22, 2017
@gibfahn
Copy link
Member

gibfahn commented Aug 22, 2017

So I see that there is a bunch of n-api stuff already, see this CI Tab with a bunch of jobs. I note that the access permissions are configured for nodejs/abi-stable-node-admins and nodejs/abi-stable-node, the latter doesn't exist any more.

So I'm guessing that the effort to add CI already started but stalled. Is that correct?

Assuming this is correct, I'd say we should:

  • Rename abi-stable-node-admins to n-api-admins to be compatible with n-api (maybe @mhdawson could do this as he's an org owner and a member of both teams).
  • Delete the existing jobs, which are based on node-test-commit
  • Copy an existing "pull node, npm install, npm test" project, like node-report or citgm.
  • Give n-api run access, and n-api-admins configure access (and also @refack so he can help out).

@nodejs/n-api can someone confirm that this is correct?

@gabrielschulhof
Copy link
Author

That tab was for the abi-stable-node fork while we were preparing the initial N-API PR. Now that N-API is in, I don't think we need it anymore. @mhdawson knows more, but that's my general impression.

OTOH, node-addon-api is a separate npm package which will remain separate, so we will continue to need separate CI for it.

@mhdawson
Copy link
Member

There are some jobs in there that are still needed and that we are working on.

The naming of the admins is because the team was renamed, since Sampson has been able to edit its possible that it is respecting both names

I'll delete the jobs we don't need

@mhdawson
Copy link
Member

I still see the team which is called:
abi-stable-node-admins which matches what the job is configured for.

@mhdawson
Copy link
Member

The name also matches the repo abi-stable-node, and host jobs that test modules that test both modules that use the node-addon-api and which use the N-API directly.

@mhdawson
Copy link
Member

I'll create a new job which is configured like the others and add refack to the admins team so that he can help configure.

@mhdawson
Copy link
Member

The other team though was renamed, at one point so I guess the admins one should match and it has less visible impact.

@mhdawson
Copy link
Member

mhdawson commented Aug 22, 2017

Reconfigured existing jobs
Added this job as a clone of the node-report one
Added refack as n-api-admins so he can configure job.

@mhdawson
Copy link
Member

@refack let us know if you need anything else.

@refack
Copy link
Contributor

refack commented Aug 22, 2017

@refack let us know if you need anything else.

Will do. Thanks!

@mhdawson
Copy link
Member

Thank you for volunteering :). I really like to have as much CI as possible and having somebody to help build it out is great.

@mhdawson
Copy link
Member

oops missed pasting in the link to the new job. Its: https://ci.nodejs.org/view/x%20-%20Abi%20stable%20module%20API/job/node-test-node-addon-api/

@maclover7
Copy link
Contributor

https://ci.nodejs.org/view/x%20-%20Abi%20stable%20module%20API/job/node-test-node-addon-api/ has been created per above, going to close for now since this seems largely solved (and the job is green!)

@refack refack removed their assignment Jun 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants