-
Notifications
You must be signed in to change notification settings - Fork 56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Errors from gax layer #1090
feat: Errors from gax layer #1090
Conversation
// netServer.listen(port); | ||
netServer.listen(`localhost:${port}`); | ||
} | ||
describe('Ensure server shuts down properly when destroyed', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I add a before hook with the following in it:
checkPort(inputPort, false, done);
the assert.notEqual(err.code, 'EADDRINUSE');
fails, but I am not sure why at this time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed the way this worked entirely to use a third party dev dependency.
…into errors-from-gax-layer
…into errors-from-gax-layer # Conflicts: # package.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a nit, I would always opt for an async/await appraoch, rather than Promise.then these days.
test/util/mock-server.ts
Outdated
const inputPort = '1234'; | ||
let server: MockServer; | ||
function checkPort(port: string, inUse: boolean, callback: () => void) { | ||
tcpPortUsed.check(parseInt(port), 'localhost').then((isInUse: boolean) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my advice would be to always use an async/await interface when possible, I think you could replace this with:
await checkPort()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It actually looks like this now:
async function checkPort(port: string, inUse: boolean, callback: () => void) {
const isInUse: boolean = await tcpPortUsed.check(
parseInt(port),
'localhost'
);
assert.strictEqual(isInUse, inUse);
callback();
}
This PR contains tests for ensuring that errors which pass through the gax layer are presented to the user properly using unit tests from a grpc service.