FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

build: split CI rules in Makefile by joaocgreis · Pull Request #7317 · nodejs/node · GitHub

/ node Public

build: split CI rules in Makefile - #7317

Closed
joaocgreis wants to merge 3 commits into
nodejs:masterfrom
JaneaSystems:joaocgreis-G6F-ci-targets
ConversationCommits (3)ChecksFiles changed
Closed

build: split CI rules in Makefile#7317
joaocgreis wants to merge 3 commits into
nodejs:masterfrom
JaneaSystems:joaocgreis-G6F-ci-targets

Conversation

joaocgreis commented Jun 16, 2016 •
edited
Loading

Copy link
Copy Markdown
Member
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 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 architecture. labels Jun 16, 2016

cjihrig commented Jun 16, 2016

Copy link
Copy Markdown
Contributor

LGTM

2 similar comments

mhdawson commented Jun 16, 2016

Copy link
Copy Markdown
Member

LGTM

santigimeno commented Jun 16, 2016

Copy link
Copy Markdown
Member

LGTM

Fishrock123 commented Jun 16, 2016

Copy link
Copy Markdown
Contributor

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

joaocgreis commented Jun 16, 2016

Copy link
Copy Markdown
Member Author

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

Fishrock123 reviewed Jun 17, 2016
View reviewed changes
Comment thread Makefile Outdated

Fishrock123 Jun 17, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

run-ci: build-ci?

jasnell commented Jun 20, 2016

Copy link
Copy Markdown
Member

LGTM

joaocgreis commented Jun 21, 2016

Copy link
Copy Markdown
Member Author

@Fishrock123 LGTY?

joaocgreis commented Jun 23, 2016

Copy link
Copy Markdown
Member Author

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

Fishrock123 reviewed Jun 24, 2016
View reviewed changes
Comment thread Makefile Outdated

Fishrock123 Jun 24, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

not | build-addons?

joaocgreis Jun 24, 2016

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 commented Jun 24, 2016

Copy link
Copy Markdown
Contributor

LGTM with a question

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.
fixup
d7c0983
fixup comments
0b041f6
joaocgreis force-pushed the joaocgreis-G6F-ci-targets branch from e33a24b to 0b041f6 Compare June 24, 2016 14:19

joaocgreis commented Jun 24, 2016

Copy link
Copy Markdown
Member Author

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

rvagg commented Jun 27, 2016

Copy link
Copy Markdown
Member

lgtm and 👍 on immediate backporting to -staging branches too

joaocgreis added a commit that referenced this pull request Jul 2, 2016
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 <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
PR-URL: #7317
joaocgreis added a commit that referenced this pull request Jul 2, 2016
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 <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
PR-URL: #7317

joaocgreis commented Jul 2, 2016

Copy link
Copy Markdown
Member Author

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 commented Jul 2, 2016

Copy link
Copy Markdown
Member Author

Adjusted CI to use the new targets. Test runs:

joaocgreis closed this Jul 2, 2016
Fishrock123 pushed a commit that referenced this pull request Jul 5, 2016
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 <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
PR-URL: #7317
Fishrock123 mentioned this pull request Jul 5, 2016
Merged
MylesBorins pushed a commit that referenced this pull request Jul 11, 2016
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 <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
PR-URL: #7317
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
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 <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
PR-URL: #7317
MylesBorins mentioned this pull request Jul 12, 2016
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
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 architecture. 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

Back | FazBrowse Home | New Git URL