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

build: split CI rules in Makefile #7317

Closed

Conversation

joaocgreis
Copy link
Member

@joaocgreis joaocgreis commented Jun 16, 2016

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

LGTM

@santigimeno
Copy link
Member

LGTM

@Fishrock123
Copy link
Contributor

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

@joaocgreis
Copy link
Member Author

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

$(PYTHON) ./configure $(CONFIG_FLAGS)
$(MAKE)

run-ci:
Copy link
Contributor

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

@Fishrock123 LGTY?

@joaocgreis
Copy link
Member Author

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

CI_NATIVE_SUITES := addons
CI_JS_SUITES := doctool known_issues message parallel pseudo-tty sequential

test-ci-native: | test/addons/.buildstamp
Copy link
Contributor

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

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

LGTM with a question

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
Copy link
Member Author

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
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
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

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

Adjusted CI to use the new targets. Test runs:

Fishrock123 pushed a commit that referenced this pull request Jul 5, 2016
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
MylesBorins pushed a commit that referenced this pull request Jul 11, 2016
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
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

8 participants