Skip to content

build: split CI rules in Makefile #7317

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.

Sign up for GitHub

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

Jump to bottom
Closed
joaocgreis wants to merge 3 commits into from
Closed

build: split CI rules in Makefile #7317

joaocgreis wants to merge 3 commits into from

Conversation

@joaocgreis
Copy link
Member

@joaocgreis joaocgreis commented Jun 16, 2016 •
edited
Loading

Checklist
  • make -j4 test (UNIX) or vcbuild test nosign (Windows) passes
  • the commit message follows commit guidelines
Affected core subsystem(s)

build

Description of change

Some CI jobs compile Node and run the tests on different machines. This change enables collaborators to have finer control over what runs on these jobs, such as the exact suites to run. The test-ci rule was split into js and native, to allow for addons to be compiled only on the machines that are going to run them.

CI run for this change: https://ci.nodejs.org/view/All/job/node-test-commit/3754/
CI run of updated binary ARM job: https://ci.nodejs.org/view/All/job/reis-test-binary-arm/1/
This adds doctool, known_issues and pseudo-tty test suites.

No change is needed for Windows, since the current vcbuild is usable by the Windows fanned job.

‼️ @nodejs/lts This should land in all branches simultaneously. After the review runs its course here, I can land this in all *-staging branches. Is that ok?

cc @nodejs/jenkins-admins @nodejs/build @nodejs/testing

@joaocgreis joaocgreis added build Issues and PRs related to build files or the CI. test Issues and PRs related to the tests. arm Issues and PRs related to the ARM platform. labels Jun 16, 2016
@cjihrig
Copy link
Contributor

cjihrig commented Jun 16, 2016

LGTM

2 similar comments
@mhdawson
Copy link
Member

mhdawson commented Jun 16, 2016

LGTM

@santigimeno
Copy link
Member

santigimeno commented Jun 16, 2016

LGTM

@Fishrock123
Copy link
Contributor

Fishrock123 commented Jun 16, 2016

Is the CI going to be back-compat so that older versions can still be tested if need be?

@joaocgreis
Copy link
Member Author

joaocgreis commented Jun 16, 2016

@Fishrock123 test-commit-arm-fanned will not be compatible, will always fail. Others will be unaffected.

Fishrock123
Fishrock123 reviewed Jun 17, 2016
View reviewed changes
Makefile Outdated
Copy link
Contributor

@Fishrock123 Fishrock123 Jun 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

run-ci: build-ci?

@jasnell
Copy link
Member

jasnell commented Jun 20, 2016

LGTM

@joaocgreis
Copy link
Member Author

joaocgreis commented Jun 21, 2016

@Fishrock123 LGTY?

@joaocgreis
Copy link
Member Author

joaocgreis commented Jun 23, 2016

Will land tomorrow and make the necessary adjustments in CI over the weekend, if there are no objections.

Fishrock123
Fishrock123 reviewed Jun 24, 2016
View reviewed changes
Makefile Outdated
Copy link
Contributor

@Fishrock123 Fishrock123 Jun 24, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not | build-addons?

Copy link
Member Author

@joaocgreis joaocgreis Jun 24, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

build-addons triggers compilation of the node executable. This cannot happen because this target will run on the test machine, after the cross-compiled binary is unpacked, and make will recompile everything.

@Fishrock123
Copy link
Contributor

Fishrock123 commented Jun 24, 2016

LGTM with a question

@joaocgreis
build: split CI rules in Makefile
eedad89 
Some CI jobs compile Node and run the tests on different machines.
This change enables collaborators to have finer control over what runs
on these jobs, such as the exact suites to run. The test-ci rule was
split into js and native, to allow for addons to be compiled only on
the machines that are going to run them.
@joaocgreis
fixup
d7c0983
@joaocgreis
fixup comments
0b041f6
@joaocgreis joaocgreis force-pushed the joaocgreis-G6F-ci-targets branch from e33a24b to 0b041f6 Compare June 24, 2016 14:19
@joaocgreis
Copy link
Member Author

joaocgreis commented Jun 24, 2016

Rebased and added a commit with comments for the new test targets.

@rvagg
Copy link
Member

rvagg commented Jun 27, 2016

lgtm and 👍 on immediate backporting to -staging branches too

joaocgreis added a commit that referenced this pull request Jul 2, 2016
@joaocgreis
build: split CI rules in Makefile
cf6ebd5 
Some CI jobs compile Node and run the tests on different machines.
This change enables collaborators to have finer control over what runs
on these jobs, such as the exact suites to run. The test-ci rule was
split into js and native, to allow for addons to be compiled only on
the machines that are going to run them.

Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
PR-URL: #7317
joaocgreis added a commit that referenced this pull request Jul 2, 2016
@joaocgreis
build: split CI rules in Makefile
7cbbec5 
Some CI jobs compile Node and run the tests on different machines.
This change enables collaborators to have finer control over what runs
on these jobs, such as the exact suites to run. The test-ci rule was
split into js and native, to allow for addons to be compiled only on
the machines that are going to run them.

Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
PR-URL: #7317
@joaocgreis
Copy link
Member Author

joaocgreis commented Jul 2, 2016

One last CI: https://ci.nodejs.org/job/node-test-pull-request/3103/

Did not land in v0.x staging branches because those versions are not tested on ARM.

@joaocgreis
Copy link
Member Author

joaocgreis commented Jul 2, 2016

Adjusted CI to use the new targets. Test runs:

@joaocgreis joaocgreis closed this Jul 2, 2016
Fishrock123 pushed a commit that referenced this pull request Jul 5, 2016
@joaocgreis @Fishrock123
build: split CI rules in Makefile
9b5be44 
Some CI jobs compile Node and run the tests on different machines.
This change enables collaborators to have finer control over what runs
on these jobs, such as the exact suites to run. The test-ci rule was
split into js and native, to allow for addons to be compiled only on
the machines that are going to run them.

Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
PR-URL: #7317
@Fishrock123 Fishrock123 mentioned this pull request Jul 5, 2016
Merged
MylesBorins pushed a commit that referenced this pull request Jul 11, 2016
@joaocgreis
build: split CI rules in Makefile
6db4dab 
Some CI jobs compile Node and run the tests on different machines.
This change enables collaborators to have finer control over what runs
on these jobs, such as the exact suites to run. The test-ci rule was
split into js and native, to allow for addons to be compiled only on
the machines that are going to run them.

Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
PR-URL: #7317
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
@joaocgreis
build: split CI rules in Makefile
8513232 
Some CI jobs compile Node and run the tests on different machines.
This change enables collaborators to have finer control over what runs
on these jobs, such as the exact suites to run. The test-ci rule was
split into js and native, to allow for addons to be compiled only on
the machines that are going to run them.

Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
PR-URL: #7317
@MylesBorins MylesBorins mentioned this pull request Jul 12, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

arm Issues and PRs related to the ARM platform. build Issues and PRs related to build files or the CI. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@joaocgreis @cjihrig @mhdawson @santigimeno @Fishrock123 @jasnell @rvagg @MylesBorins