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: fix test-readline-interface short delay #14677

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
8 participants
@Azard
Contributor

Azard commented Aug 7, 2017

Previous unit test delay is too short for parallel test on raspberry pi, it will fail sometimes.
This PR use common.platformTimeout and widen the time gap, a patch of #13497

Refs: #14674

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

readline

  • test-readline-interface

@mscdex mscdex added the readline label Aug 7, 2017

@refack

refack approved these changes Aug 7, 2017

@cjihrig

cjihrig approved these changes Aug 7, 2017

@Trott Trott referenced this pull request Aug 8, 2017

Closed

fix test-readline-interface #14681

2 of 2 tasks complete
@jasnell

jasnell approved these changes Aug 8, 2017

@Trott

This comment has been minimized.

Show comment
Hide comment
@Trott

Trott approved these changes Aug 8, 2017

@Trott

This comment has been minimized.

Show comment
Hide comment
@Trott

This comment has been minimized.

Show comment
Hide comment
@Trott

Trott Aug 8, 2017

Member

Stress test on master (should show failures): https://ci.nodejs.org/job/node-stress-single-test-pi1-fanned/27/

Stress test on this PR (should be green): https://ci.nodejs.org/job/node-stress-single-test-pi1-fanned/28/

Member

Trott commented Aug 8, 2017

Stress test on master (should show failures): https://ci.nodejs.org/job/node-stress-single-test-pi1-fanned/27/

Stress test on this PR (should be green): https://ci.nodejs.org/job/node-stress-single-test-pi1-fanned/28/

@Trott

This comment has been minimized.

Show comment
Hide comment
@Trott

Trott Aug 8, 2017

Member

CI is good, stress tests are great, change is small, will help flip one of our CI tasks from red to green, 4 approvals, so I'm going to fast-track this...

Member

Trott commented Aug 8, 2017

CI is good, stress tests are great, change is small, will help flip one of our CI tasks from red to green, 4 approvals, so I'm going to fast-track this...

@Trott

This comment has been minimized.

Show comment
Hide comment
@Trott

Trott Aug 8, 2017

Member

Landed in f11379d

Member

Trott commented Aug 8, 2017

Landed in f11379d

@Trott Trott closed this Aug 8, 2017

Trott added a commit to Trott/io.js that referenced this pull request Aug 8, 2017

test: fix test-readline-interface
Previous unit test delay is too short for parallel test on raspberry pi,
it will fail sometimes.  This PR use common.platformTimeout and widen
the time gap.

PR-URL: nodejs#14677
Ref: nodejs#14674
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

addaleax added a commit that referenced this pull request Aug 10, 2017

test: fix test-readline-interface
Previous unit test delay is too short for parallel test on raspberry pi,
it will fail sometimes.  This PR use common.platformTimeout and widen
the time gap.

PR-URL: #14677
Ref: #14674
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

icarter09 added a commit to icarter09/node that referenced this pull request Aug 12, 2017

test: fix test-readline-interface
Previous unit test delay is too short for parallel test on raspberry pi,
it will fail sometimes.  This PR use common.platformTimeout and widen
the time gap.

PR-URL: nodejs#14677
Ref: nodejs#14674
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

@addaleax addaleax referenced this pull request Aug 13, 2017

Merged

v8.4.0 proposal #14811

@Azard Azard referenced this pull request Aug 17, 2017

Closed

[v6.x backport] readline: remove max limit of crlfDelay #14899

4 of 4 tasks complete

MylesBorins added a commit that referenced this pull request Sep 19, 2017

test: fix test-readline-interface
Previous unit test delay is too short for parallel test on raspberry pi,
it will fail sometimes.  This PR use common.platformTimeout and widen
the time gap.

PR-URL: #14677
Ref: #14674
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

@MylesBorins MylesBorins referenced this pull request Sep 20, 2017

Merged

v6.11.4 proposal #15506

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment