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: allow test-ci to run tests in parallel #6208

Merged

Conversation

jbergstroem
Copy link
Member

Checklist
  • tests and code linting passes
  • the commit message follows commit guidelines
Affected core subsystem(s)

build

Description of change

Run tests in parallel if the environment variable JOBS (which should contain a number of parallel jobs) is set.

I'd like to expand this to the actual build steps as well (and possibly even outside of the ci scope), but it requires careful testing since it seems to break here and there for some jobs.

/cc=@nodejs/build, @Trott, @bnoordhuis.

@jbergstroem jbergstroem added the build Issues and PRs related to build files or the CI. label Apr 15, 2016
@rvagg
Copy link
Member

rvagg commented Apr 15, 2016

yay! lgtm

@jbergstroem jbergstroem mentioned this pull request Apr 15, 2016
4 tasks
@jbergstroem
Copy link
Member Author

@MylesBorins
Copy link
Contributor

LGTM if CI is green

@jbergstroem jbergstroem force-pushed the feature/parallel-jobs-in-makefile branch from af394c3 to 9b99658 Compare April 15, 2016 04:51
@rvagg
Copy link
Member

rvagg commented Apr 15, 2016

CI looking good but the speed increase isn't really evident

@jbergstroem
Copy link
Member Author

@rvagg: most vm's doesn't have JOBS= set. The lint job finishes in <30sec though (fluctuated between 55sec and 1m30s before @mscdex's #5638): https://ci.nodejs.org/job/node-test-linter/2051/

@Trott
Copy link
Member

Trott commented Apr 15, 2016

LGTM

1 similar comment
@bnoordhuis
Copy link
Member

LGTM

Run tests in parallel if the environment variable JOBS
(which should contain a number of parallel jobs) is set.

PR-URL: nodejs#6208
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@jbergstroem jbergstroem force-pushed the feature/parallel-jobs-in-makefile branch from 9b99658 to f49a1d0 Compare April 16, 2016 01:16
@jbergstroem jbergstroem merged commit f49a1d0 into nodejs:master Apr 16, 2016
@lpsuerj lpsuerj mentioned this pull request Apr 16, 2016
MylesBorins pushed a commit that referenced this pull request Apr 20, 2016
Run tests in parallel if the environment variable JOBS
(which should contain a number of parallel jobs) is set.

PR-URL: #6208
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Apr 20, 2016
MylesBorins pushed a commit that referenced this pull request Apr 20, 2016
Run tests in parallel if the environment variable JOBS
(which should contain a number of parallel jobs) is set.

PR-URL: #6208
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@MylesBorins
Copy link
Contributor

@jbergstroem lts?

@jbergstroem
Copy link
Member Author

@thealphanerd sure.

MylesBorins pushed a commit that referenced this pull request Apr 21, 2016
Run tests in parallel if the environment variable JOBS
(which should contain a number of parallel jobs) is set.

PR-URL: #6208
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
joelostrowski pushed a commit to joelostrowski/node that referenced this pull request Apr 25, 2016
Run tests in parallel if the environment variable JOBS
(which should contain a number of parallel jobs) is set.

PR-URL: nodejs#6208
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
jasnell pushed a commit that referenced this pull request Apr 26, 2016
Run tests in parallel if the environment variable JOBS
(which should contain a number of parallel jobs) is set.

PR-URL: #6208
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@MylesBorins
Copy link
Contributor

@jbergstroem this is not landing cleanly on v4.x-staing

will likely need to be backported manually

@MylesBorins
Copy link
Contributor

ping @jbergstroem

@jbergstroem
Copy link
Member Author

Since #5638 is not landing I reckon we can skip this too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants