Skip to content

inspector: fix tests on Windows - #8528

Closed
eugeneo wants to merge 1 commit into
nodejs:masterfrom
eugeneo:tests
Closed

inspector: fix tests on Windows#8528
eugeneo wants to merge 1 commit into
nodejs:masterfrom
eugeneo:tests

Conversation

@eugeneo

@eugeneoeugeneo commented Sep 14, 2016

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

Inspector protocol implementation & test

Description of change

Should help with #8034

@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. inspector Issues and PRs related to the V8 inspector protocol labels Sep 14, 2016
@mscdexmscdex added the windows Issues and PRs related to the Windows platform. label Sep 14, 2016
Comment threadtest/cctest/test_inspector_socket.cc Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you write it as uv_stream_t* (no space) while you're here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Updated this location and several similar ones.

@bnoordhuis

Copy link
Copy Markdown
Member

LGTM

@eugeneo

Copy link
Copy Markdown
ContributorAuthor

Thank you for the review. Commit was updated.

@eugeneo

Copy link
Copy Markdown
ContributorAuthor

@ofrobots

@ofrobots

Copy link
Copy Markdown
Contributor

@ofrobots

Copy link
Copy Markdown
Contributor

The AIX build failure looked mysterious; relaunched: https://ci.nodejs.org/job/node-test-commit-aix/861/.

@ofrobots

Copy link
Copy Markdown
Contributor

CI is green enough. AIX rebuild was successful and the arm-fanned failure looks like a bot issue.

@ofrobots

Copy link
Copy Markdown
Contributor

Landed as 6f9157f.

ofrobots pushed a commit that referenced this pull request Sep 16, 2016
Should help with #8034.
PR-URL: #8528
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
@eugeneo
eugeneo deleted the tests branch September 16, 2016 17:16
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Sep 28, 2016
Should help with nodejs#8034.
PR-URL: nodejs#8528
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
MylesBorins pushed a commit that referenced this pull request Sep 30, 2016
Should help with #8034.
PR-URL: #8528
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++Issues and PRs that require attention from people who are familiar with C++.inspectorIssues and PRs related to the V8 inspector protocolwindowsIssues and PRs related to the Windows platform.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@eugeneo@bnoordhuis@ofrobots@mscdex@MylesBorins@nodejs-github-bot