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

Document CI jobs #12021

Closed
gibfahn opened this issue Mar 24, 2017 · 14 comments
Closed

Document CI jobs #12021

gibfahn opened this issue Mar 24, 2017 · 14 comments
Labels
doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors.

Comments

@gibfahn
Copy link
Member

gibfahn commented Mar 24, 2017

  • Subsystem: doc, build

See nodejs/help#548, we should briefly document the jobs we have in CI, probably in the COLLABORATOR_GUIDE.

https://ci.nodejs.org/job/node-stress-single-test/

cc/ @vsemozhetbyt

@gibfahn gibfahn added doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. labels Mar 24, 2017
@vsemozhetbyt
Copy link
Contributor

@gibfahn I am not sure for now how to do this, need some experiments (the first attempt has failed, maybe I've configured something wrong). Maybe it would be better if anybody with more experience here documents this.

@joyeecheung
Copy link
Member

joyeecheung commented Mar 25, 2017

Er, I am not sure if this is actually a good first contribution? (unless mentors are available)

@gibfahn
Copy link
Member Author

gibfahn commented Mar 25, 2017

@joyeecheung mentors are available 😄

I think what we need is a list of the ci jobs that collaborators (or any other contributor) need to know about. ci.nodejs.org is open to anyone to look at.

The jobs I can think of are:

For each one we'd need a link and a one sentence description of what they are for. Just so that new collaborators get told this as part of Onboarding, instead of having to find out by chance later.

@vsemozhetbyt
Copy link
Contributor

@gibfahn It seems the https://ci.nodejs.org/job/node-test-linter/ is also worth documenting for PRs concerning files that are not relevant for builds (like benchmarks).

@morrme
Copy link
Contributor

morrme commented Mar 27, 2017

I'm interested in helping with this, with support from a mentor.

@gibfahn
Copy link
Member Author

gibfahn commented Mar 27, 2017

@morrme Thanks!

So this info should be added to the CI Testing section of either CONTRIBUTING.md or the onboarding.md doc, depending on whether you think it'd be useful for people who aren't collaborators (for example, if you think this info would be useful for you as you read CONTRIBUTING.md?)

We probably want a sentence saying that our test jobs run from ci.nodejs.org, and then give links to some of the most commonly used jobs (i.e. those mentioned above).

A guide to the mechanics of raising a PR can be found in CONTRIBUTING.md as well.

If you need any help, just comment in here. A PR can also be raised for feedback before it's finished if you'd like.

@morrme
Copy link
Contributor

morrme commented Mar 27, 2017

@gibfahn Thanks! It is definitely important information for a collaborator and I personally find it interesting as a contributor as well.

So far, there are 5 tests listed.

I found a little information about node-stress_single-test from issue #3854 . Are there any other notes I can use to come up with the descriptions?

@gibfahn
Copy link
Member Author

gibfahn commented Mar 27, 2017

@morrme I think all you'd really need to say is that the job allows you to specify a set of tests to pass to tools/test.py, and that there's a repeat option to run a test or tests in a loop. A couple of examples might be helpful as well (cc/ @Trott, we were discussing this in a PR somewhere, can't remember where).

@Trott
Copy link
Member

Trott commented Mar 28, 2017

@morrme I think all you'd really need to say is that the job allows you to specify a set of tests to pass to tools/test.py, and that there's a repeat option to run a test or tests in a loop. A couple of examples might be helpful as well (cc/ @Trott, we were discussing this in a PR somewhere, can't remember where).

For an initial pass, the repeat info can be left off. We can add it later (or not). The main thing about node-stress-single-test is that it's designed to allow one to run a single test over and over on a specific platform to confirm that the test is reliable.

@morrme
Copy link
Contributor

morrme commented Mar 31, 2017

@gibfahn OK so for the ones you've marked as "possibly already documented", I cannot seem to find anything on these.

And for @vsemozhetbyt 's suggestion of https://ci.nodejs.org/job/node-test-linter/ : what would be a good description?

@gibfahn
Copy link
Member Author

gibfahn commented Mar 31, 2017

OK so for the ones you've marked as "possibly already documented", I cannot seem to find anything on these.

Okay, then please do document them as well.

https://ci.nodejs.org/job/node-test-linter/ : what would be a good description?

Maybe something like:

The node-test-linter only runs the linter targets, which is useful for changes that only affect comments or documentation.

@morrme
Copy link
Contributor

morrme commented Apr 2, 2017

OK ongoing notes:

node-stress_single-test

allows you to specify a set of tests to pass to tools/test.py, and that there's a repeat option to run a test or tests in a loop. A couple of examples might be helpful as well
it's designed to allow one to run a single test over and over on a specific platform to confirm that the test is reliable.

node-test-linter

The node-test-linter only runs the linter targets, which is useful for changes that only affect comments or documentation.


For node-test-pull-request
I have found #2263
Is this info still valid/current? anything to add?

There is also information there about node-test-commit but I'm still hoping to find a better description.

For citgm-smoker

Possibly use some of the NPM description: https://www.npmjs.com/package/citgm

citgm is a simple tool for pulling down an arbitrary module from npm and testing it using a specific version of the node runtime.

The Node.js project uses citgm to smoketest our releases and controversial changes.

@gibfahn
Copy link
Member Author

gibfahn commented Apr 6, 2017

@morrme Sorry for forgetting about this! What you've got looks good.

For node-test-pull-request, maybe something like this would be helpful:

node-test-pull-request is the standard CI run we do to check Pull Requests. It triggers node-test-commit, which runs the build-ci and test-ci targets on all supported platforms.


I would suggest you order it:

  • node-test-pull-request
  • node-test-linter
  • citgm-smoker
  • node-stress-test

as that's (I think) the order in which they get used.


For citgm-smoker, maybe something like:

The citgm-smoker job uses CitGM to allow you to run npm install && npm test on a large selection of common modules. This is useful to check whether a change will cause breakage in the ecosystem. To test node ABI changes you can run citgm-abi-smoker.


I think what you have is ready for a PR, if you submit one then you should get lots of feedback from other collaborators, and we can hopefully get it landed.

@morrme
Copy link
Contributor

morrme commented Apr 20, 2017

@gibfahn did not forget about this! i think i've been over-obsessing about wording and i'm sure there will be changes during the review process, so I'll send over what I have.

anchnk pushed a commit to anchnk/node that referenced this issue May 19, 2017
PR-URL: nodejs#12555
Fixes: nodejs#12021
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit that referenced this issue Jun 22, 2017
PR-URL: #12555
Fixes: #12021
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit that referenced this issue Jul 11, 2017
PR-URL: #12555
Fixes: #12021
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants