Add dynamic port allocation #913
Conversation
|
@chrisvfritz this would break e2e as nighwatch needs to know the port of the devserver to run the tests. |
|
Should I add a condition for testing environment? |
|
That would be good, yes. |
|
@adi518 @LinusBorg Another potential option is we could set an environment variable here with the port number, that is later read by Nightwatch. |
|
Great idea. |
|
I set the variable as suggested by @chrisvfritz. |
|
@LinusBorg How's this looking to you? |
|
Will take a final look at the weekend when I'm back from portugal |
|
@LinusBorg Sorry, forgot you were on vacation! 😅 |
| var uri = 'http://localhost:' + port | ||
| console.log('> Listening at ' + uri + '\n') | ||
| // when env is testing, don't need open it | ||
| if (autoOpenBrowser && process.env.NODE_ENV !== 'testing') { |
There was a problem hiding this comment.
When we talked about adding a check for 'testing' environment, I didn't think about skipping the opn(uri) - I was thinking about skipping the whole portfinder logic.
The point is: for e2e tests, we currently try to access the dev-server through the port that is defined in the config (see this line). If the portfinder starts the def-server at a different port because that port is in use, e2e tests will connect to that wrong port anyway.
So either we find a way to tell the e2e test runner about the new found port, or we have to skip portfinder completely hen running e2e tests.
There was a problem hiding this comment.
I thought we already did that. That is, updating the environment variable read by the e2e config.
There was a problem hiding this comment.
Oh, right - sorry, I missed that.
* Add dynamic port allocation * Add error handling to port allocation * Remove forgotten listen statement from dev-server.js * Set dynamically allocated port to environment variable `PORT`
Scans for an open port starting with the port set in
config.