Skip to content

test: more robust check for location of node.exe - #12120

Closed
refack wants to merge 1 commit into
nodejs:masterfrom
refack:patch-1
Closed

test: more robust check for location of node.exe#12120
refack wants to merge 1 commit into
nodejs:masterfrom
refack:patch-1

Conversation

@refack

Copy link
Copy Markdown
Contributor
Checklist
  • vcbuild test (Windows) passes
  • commit message follows [commit guidelines][]
Affected core subsystem(s)

windows, test

@nodejs-github-botnodejs-github-bot added the tools Issues and PRs related to the tools directory. label Mar 29, 2017
@refack

Copy link
Copy Markdown
ContributorAuthor

I'm compiling for windows & ubuntu on the same dir so I have out/Release directory, but the exe is obviously not there. (will also happen when mix-compiling with ninja and MSVS)

@bnoordhuis

Copy link
Copy Markdown
Member

Can you update the commit log to explain how it's more robust? It's not immediately evident from looking at the diff.

@refack

Copy link
Copy Markdown
ContributorAuthor

@bnoordhuis updated. Also added explicit error.

@bnoordhuisbnoordhuis left a comment

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.

LGTM, I think, modulo comment.

Comment threadtools/test.py 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 include name in the message and use single quotes?

look for the actual produced `exe` not just the directory
@mscdexmscdex added the windows Issues and PRs related to the Windows platform. label Mar 29, 2017
@gibfahn

Copy link
Copy Markdown
Member

cc/ @nodejs/platform-windows

@jasnell

Copy link
Copy Markdown
Member

jasnell pushed a commit that referenced this pull request Apr 4, 2017
look for the actual produced `exe` not just the directory
PR-URL: #12120
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com>
@jasnell

Copy link
Copy Markdown
Member

Landed in 394b6ac

@jasnelljasnell closed this Apr 4, 2017
@refack

Copy link
Copy Markdown
ContributorAuthor

Yeah broke the slump ;)

@jasnelljasnell mentioned this pull request Apr 4, 2017
@refack
refack deleted the patch-1 branch April 4, 2017 21:33
italoacasas pushed a commit to italoacasas/node that referenced this pull request Apr 10, 2017
look for the actual produced `exe` not just the directory
PR-URL: nodejs#12120
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com>
@italoacasasitaloacasas mentioned this pull request Apr 10, 2017
2 tasks
MylesBorins pushed a commit that referenced this pull request Apr 18, 2017
look for the actual produced `exe` not just the directory
PR-URL: #12120
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com>
MylesBorins pushed a commit that referenced this pull request Apr 19, 2017
look for the actual produced `exe` not just the directory
PR-URL: #12120
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com>
@MylesBorinsMylesBorins mentioned this pull request Apr 19, 2017
andrew749 pushed a commit to michielbaird/node that referenced this pull request Jul 19, 2017
look for the actual produced `exe` not just the directory
PR-URL: nodejs/node#12120
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

toolsIssues and PRs related to the tools directory.windowsIssues and PRs related to the Windows platform.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@refack@bnoordhuis@gibfahn@jasnell@seishun@mscdex@MylesBorins@nodejs-github-bot