Skip to content

Conversation

ghost
Copy link

@ghost ghost commented Jul 27, 2016

I had a PORT environment variable set to 80, and runtests-browser used that in some cases but not in others. Turns out it currently only works with a port of 8888, see #9987.

@ghost ghost force-pushed the fix_runtests_browser branch from d7e918e to 2873eea Compare July 27, 2016 15:55
@ghost ghost mentioned this pull request Jul 27, 2016
@weswigham
Copy link
Member

weswigham commented Jul 27, 2016

@Andy-MS If you're going to limit it to just port 8888, why take an argument at all? I don't think too many people are clamoring for this to be configurable.

@ghost
Copy link
Author

ghost commented Jul 27, 2016

You might have a good point. Do you think we should just get rid of the "port" argument in gulpfile/jakefile? We should first make sure that no one is using it.
EDIT: Of course no one should be using it because it was broken before...

@sandersn
Copy link
Member

Yeah, I'd just hardcode it to 8888 unless there are any objections. I don't have any. :)

The only scenario I can think of is debugging TypeScript while running some other kind of local server. But the person that runs into that corner case can just hard code to some other port and recompile — I don't think it's a regularly occurring case.

@@ -10,37 +10,34 @@ import os = require("os");
/// Command line processing ///

if (process.argv[2] == '--help') {
console.log('Runs a node server on port 8888 by default, looking for tests folder in the current directory\n');
console.log('Runs a node server on port 8888, looking for tests folder in the current directory\n');
console.log('Syntax: node nodeServer.js [port] [typescriptEnlistmentDirectory] [tests] [--browser] [--verbose]\n');
Copy link
Member

Choose a reason for hiding this comment

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

need to get rid of [port] here too

@sandersn
Copy link
Member

👍

@ghost ghost changed the title Remove hardcoded port, use the custom port Always use a hardcoded port Jul 29, 2016
@ghost ghost merged commit 878cf85 into master Jul 29, 2016
@ghost ghost deleted the fix_runtests_browser branch July 29, 2016 12:54
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants