Skip to content

test: fix truncation of argv #12110

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 1 commit into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Mar 29, 2017

Currently argv_[1] and argv_[2] are getting truncated by one character
because of an incorrect addition of one to account for the null
character. I only noticed this when working on #12087, but that fix
will probably not get included in favor of a JavaScript test so I'm
adding this separate commit for it.

Refs: #12087

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test

Currently argv_[1] and argv_[2] are getting truncated by one character
because of an incorrect addition of one to account for the null
character. I only noticed this when working on nodejs#12087, but that fix
will probably not get included in favor of a JavaScript test so I'm
adding this separate commit for it.

Refs: nodejs#12087
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Mar 29, 2017
@mscdex mscdex added the c++ Issues and PRs that require attention from people who are familiar with C++. label Mar 29, 2017
danbev added a commit to danbev/node that referenced this pull request Mar 31, 2017
Currently argv_[1] and argv_[2] are getting truncated by one character
because of an incorrect addition of one to account for the null
character. I only noticed this when working on nodejs#12087, but that fix
will probably not get included in favor of a JavaScript test so I'm
adding this separate commit for it.

Refs: nodejs#12087
PR-URL: nodejs#12110
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@danbev
Copy link
Contributor Author

danbev commented Mar 31, 2017

Landed in 92de91d

@danbev danbev closed this Mar 31, 2017
@danbev danbev deleted the node_test_fixture_args_fix branch March 31, 2017 05:43
@jasnell jasnell mentioned this pull request Apr 4, 2017
@gibfahn gibfahn mentioned this pull request Jun 15, 2017
3 tasks
@gibfahn
Copy link
Member

gibfahn commented Jun 17, 2017

@danbev should this land on v6.x?

@MylesBorins
Copy link
Contributor

ping @danbev

4 similar comments
@MylesBorins
Copy link
Contributor

ping @danbev

@MylesBorins
Copy link
Contributor

ping @danbev

@MylesBorins
Copy link
Contributor

ping @danbev

@MylesBorins
Copy link
Contributor

ping @danbev

@danbev
Copy link
Contributor Author

danbev commented Jan 11, 2018

@MylesBorins Sorry, can't believe I missed all of these 😞 . I'll take a look at this first thing tomorrow morning.

danbev added a commit to danbev/node that referenced this pull request Jan 12, 2018
Currently argv_[1] and argv_[2] are getting truncated by one character
because of an incorrect addition of one to account for the null
character. I only noticed this when working on nodejs#12087, but that fix
will probably not get included in favor of a JavaScript test so I'm
adding this separate commit for it.

Refs: nodejs#12087
PR-URL: nodejs#12110
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@danbev
Copy link
Contributor Author

danbev commented Jan 12, 2018

@gibfahn @MylesBorins My apologies for completely missing this. I've created a backport request against v6x-staging now.

MylesBorins pushed a commit that referenced this pull request Jan 18, 2018
Currently argv_[1] and argv_[2] are getting truncated by one character
because of an incorrect addition of one to account for the null
character. I only noticed this when working on #12087, but that fix
will probably not get included in favor of a JavaScript test so I'm
adding this separate commit for it.

Refs: #12087
Backport-PR-URL: #18113
PR-URL: #12110
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jan 24, 2018
MylesBorins pushed a commit that referenced this pull request Feb 11, 2018
Currently argv_[1] and argv_[2] are getting truncated by one character
because of an incorrect addition of one to account for the null
character. I only noticed this when working on #12087, but that fix
will probably not get included in favor of a JavaScript test so I'm
adding this separate commit for it.

Refs: #12087
Backport-PR-URL: #18113
PR-URL: #12110
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 12, 2018
Currently argv_[1] and argv_[2] are getting truncated by one character
because of an incorrect addition of one to account for the null
character. I only noticed this when working on #12087, but that fix
will probably not get included in favor of a JavaScript test so I'm
adding this separate commit for it.

Refs: #12087
Backport-PR-URL: #18113
PR-URL: #12110
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 13, 2018
Currently argv_[1] and argv_[2] are getting truncated by one character
because of an incorrect addition of one to account for the null
character. I only noticed this when working on #12087, but that fix
will probably not get included in favor of a JavaScript test so I'm
adding this separate commit for it.

Refs: #12087
Backport-PR-URL: #18113
PR-URL: #12110
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants