Skip to content

Conversation

@jrainville
Copy link
Collaborator

Fix the case where you try to test specifying a non available node or the node is off.
Moved the pingEndpoint function to utils so that I could use it in tests too.

@iurimatias iurimatias merged commit b363873 into develop Jun 19, 2018
Copy link
Collaborator

@emizzle emizzle left a comment

Choose a reason for hiding this comment

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

Looks good

req.on('upgrade', (_res, _socket, _head) => {
next();
});
const {host, port, type, protocol} = self.contractsConfig.deployment;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we handle the case where protocol is not specified in the config, ie default to http? Or possibly amend the default contracts config options to include "protocol": "http"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

pingEndpoint handles the case where protocol is undefined, but maybe setting as a default in config.js would be a good idea

if (!protocol) {
protocol = (this.simOptions.type === "rpc") ? 'http' : 'ws';
}
const endpoint = `${protocol}://${host}:${port}`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could also use utils.buildUrl or utils.buildUrlFromConfig.

@jrainville jrainville deleted the bug_fix/hanging-test-node branch June 19, 2018 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants