Skip to content

test: refactor test-debug-args #9833

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
Closed

Conversation

Trott
Copy link
Member

@Trott Trott commented Nov 29, 2016

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

test debugger

Description of change
  • indexOf() -> includes()
  • var -> const

@Trott Trott added debugger test Issues and PRs related to the tests. labels Nov 29, 2016
@Trott
Copy link
Member Author

Trott commented Nov 30, 2016

lpinca
lpinca previously approved these changes Nov 30, 2016
@lpinca
Copy link
Member

lpinca commented Nov 30, 2016

Hmm failures seem related.

@lpinca lpinca dismissed their stale review November 30, 2016 17:20

Incorrect review

Copy link
Member

@lpinca lpinca left a comment

Choose a reason for hiding this comment

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

See inline comment.


assert.notEqual(process.execArgv.indexOf('--debug-code'), -1);
assert(!process.execArgv.includes('--debug-code'));
Copy link
Member

Choose a reason for hiding this comment

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

The ! should be removed as the original check was process.execArgv.indexOf('--debug-code') != -1.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, that's certainly embarrassing. I must have tried to do a shortcut by only running the test I changed, and then pasted in the wrong file name or something. Fixing momentarily.

* indexOf() -> includes()
* var -> const
@Trott
Copy link
Member Author

Trott commented Nov 30, 2016

Fixed. Let's try again. CI: https://ci.nodejs.org/job/node-test-pull-request/5032/

Trott added a commit to Trott/io.js that referenced this pull request Dec 2, 2016
* indexOf() -> includes()
* var -> const

PR-URL: nodejs#9833
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@Trott
Copy link
Member Author

Trott commented Dec 2, 2016

Landed in cf2562b

@Trott Trott closed this Dec 2, 2016
addaleax pushed a commit that referenced this pull request Dec 5, 2016
* indexOf() -> includes()
* var -> const

PR-URL: #9833
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@Fishrock123 Fishrock123 mentioned this pull request Dec 5, 2016
2 tasks
addaleax pushed a commit to addaleax/node that referenced this pull request Dec 8, 2016
* indexOf() -> includes()
* var -> const

PR-URL: nodejs#9833
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
jmdarling pushed a commit to jmdarling/node that referenced this pull request Dec 8, 2016
* indexOf() -> includes()
* var -> const

PR-URL: nodejs#9833
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
* indexOf() -> includes()
* var -> const

PR-URL: #9833
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Dec 21, 2016
@Trott Trott deleted the update-test branch January 13, 2022 22:44
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants