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

test: use regular timeout times for ARMv8 #4248

Merged
merged 1 commit into from
Dec 14, 2015

Conversation

Fishrock123
Copy link
Contributor

ARMv8 machines are typically quite fast and likely may not need extended timeout times.

I don't have data on this but I figure it is worth a shot.

cc @nodejs/build

@Fishrock123 Fishrock123 added the test Issues and PRs related to the tests. label Dec 11, 2015
@mscdex mscdex added the arm Issues and PRs related to the ARM platform. label Dec 11, 2015
@bnoordhuis
Copy link
Member

LGTM if the CI agrees.

@jasnell
Copy link
Member

jasnell commented Dec 12, 2015

LGTM

@Trott
Copy link
Member

Trott commented Dec 13, 2015

LGTM if CI doesn't reveal anything problematic

@rvagg
Copy link
Member

rvagg commented Dec 14, 2015

Good one @Fishrock123, lgtm pending https://ci.nodejs.org/job/node-test-commit/1422/

@@ -245,10 +245,15 @@ exports.platformTimeout = function(ms) {
if (process.arch !== 'arm')
return ms;

if (process.config.variables.arm_version === '6')
const armv = process.config.variables.arm_version
Copy link
Member

Choose a reason for hiding this comment

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

missing semicolon

@cjihrig
Copy link
Contributor

cjihrig commented Dec 14, 2015

LGTM. CI seems happy, minus the missing semicolon.

ARMv8 machines are typically quite fast and likely may not need
extended timeout times.

PR-URL: nodejs#4248
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@Fishrock123 Fishrock123 merged commit 668449a into nodejs:master Dec 14, 2015
@Fishrock123 Fishrock123 deleted the armv8-timeouts branch December 14, 2015 18:09
Fishrock123 added a commit that referenced this pull request Dec 15, 2015
ARMv8 machines are typically quite fast and likely may not need
extended timeout times.

PR-URL: #4248
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@rvagg rvagg mentioned this pull request Dec 17, 2015
Fishrock123 added a commit that referenced this pull request Dec 30, 2015
ARMv8 machines are typically quite fast and likely may not need
extended timeout times.

PR-URL: #4248
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2016
ARMv8 machines are typically quite fast and likely may not need
extended timeout times.

PR-URL: #4248
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jan 19, 2016
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
ARMv8 machines are typically quite fast and likely may not need
extended timeout times.

PR-URL: nodejs#4248
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
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. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants