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: fix test-debugger-repl-break-in-module #6686

Closed
wants to merge 1 commit into from
Closed

Conversation

Trott
Copy link
Member

@Trott Trott commented May 11, 2016

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

test debugger

Description of change

The line number checks in test-debugger-repl-break-in-module were
checking for line numbers that exceed the total number of lines in the
files that were being inspected. Change the checks to match the actual
files.

@Trott Trott added debugger test Issues and PRs related to the tests. labels May 11, 2016
@santigimeno
Copy link
Member

santigimeno commented May 11, 2016

Fixes the issue for me. LGTM

]);

// -- CLEAR BREAKPOINT SET IN MODULE TO BE LOADED --
// // -- CLEAR BREAKPOINT SET IN MODULE TO BE LOADED --
Copy link
Member

Choose a reason for hiding this comment

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

Accidental change?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, accident, I'll fix it...

@bnoordhuis
Copy link
Member

Is the plan to eventually move this test to test/parallel or test/sequential?

@Trott
Copy link
Member Author

Trott commented May 11, 2016

@bnoordhuis I hadn't planned on moving them to sequential or parallel specifically, but just making sure that they get exercised in the test and test-ci tasks. Moving them to sequential is one way to do that, so that works for me. (I think parallel won't work unless we modify all the tests to use a custom port for the debugger.)

The line number checks in test-debugger-repl-break-in-module were
checking for line numbers that exceed the total number of lines in the
files that were being inspected. Change the checks to match the actual
files.
@Trott
Copy link
Member Author

Trott commented May 11, 2016

Rebased, fixed accidental comment change, force pushed.

@bnoordhuis
Copy link
Member

LGTM

I think parallel won't work unless we modify all the tests to use a custom port for the debugger.

Agreed.

@Trott
Copy link
Member Author

Trott commented May 13, 2016

@Trott
Copy link
Member Author

Trott commented May 13, 2016

Two failures in CI are a known flaky test and a definitely-unrelated test that may also be flaky.

Just to be cautious, re-running CI: https://ci.nodejs.org/job/node-test-pull-request/2625/

@Trott
Copy link
Member Author

Trott commented May 13, 2016

CI still looks good but with a pair of unrelated failures.

More caution: https://ci.nodejs.org/job/node-test-pull-request/2626/

Trott added a commit to Trott/io.js that referenced this pull request May 13, 2016
The line number checks in test-debugger-repl-break-in-module were
checking for line numbers that exceed the total number of lines in the
files that were being inspected. Change the checks to match the actual
files.

PR-URL: nodejs#6686
Reviewed-By: Ben Noorhduis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
@Trott
Copy link
Member Author

Trott commented May 13, 2016

Green that time. Landed in 9d445bc.

@Trott Trott closed this May 13, 2016
evanlucas pushed a commit that referenced this pull request May 17, 2016
The line number checks in test-debugger-repl-break-in-module were
checking for line numbers that exceed the total number of lines in the
files that were being inspected. Change the checks to match the actual
files.

PR-URL: #6686
Reviewed-By: Ben Noorhduis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jun 2, 2016
The line number checks in test-debugger-repl-break-in-module were
checking for line numbers that exceed the total number of lines in the
files that were being inspected. Change the checks to match the actual
files.

PR-URL: #6686
Reviewed-By: Ben Noorhduis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jun 2, 2016
The line number checks in test-debugger-repl-break-in-module were
checking for line numbers that exceed the total number of lines in the
files that were being inspected. Change the checks to match the actual
files.

PR-URL: #6686
Reviewed-By: Ben Noorhduis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jun 24, 2016
MylesBorins pushed a commit that referenced this pull request Jun 24, 2016
The line number checks in test-debugger-repl-break-in-module were
checking for line numbers that exceed the total number of lines in the
files that were being inspected. Change the checks to match the actual
files.

PR-URL: #6686
Reviewed-By: Ben Noorhduis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jun 24, 2016
The line number checks in test-debugger-repl-break-in-module were
checking for line numbers that exceed the total number of lines in the
files that were being inspected. Change the checks to match the actual
files.

PR-URL: #6686
Reviewed-By: Ben Noorhduis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
@Trott Trott deleted the myles branch January 13, 2022 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants