-
Notifications
You must be signed in to change notification settings - Fork 269
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
Add tests to the error handling when using multiple endpoints #545
Conversation
I pushed a small change to the error message, can you please rebase and update your tests? Thanks! |
Sure, done |
test/login_test.exs
Outdated
|
||
assert_start_and_killed(opts ++ context[:options]) | ||
end) =~ | ||
~r'\*\* \(Postgrex\.Error\) failed to establish connection to multiple endpoints\:\n\n \* doesntexist\:5432\: \(DBConnection\.ConnectionError\) tcp connect \(doesntexist\:5432\)\: non\-existing domain \- \:nxdomain\n \* localhost\:5555\: \(DBConnection\.ConnectionError\) tcp connect \(localhost\:5555\)\: connection refused \- \:econnrefused' |
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 don't think those need to be regexes, as using a string will also be a submatch. Can we use strings instead? They should be easier to read. :) Thanks!
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.
Indeed. done :)
Here is the current output:
Is the double newline character after "multiple endpoints:" intended? Wouldn't one newline be enough? |
@chaodhib it is intentional, most Elixir exceptions use that standard. :) |
@josevalim good to know. thank you :) |
This PR is a minor follow up to #544. I thought it would be better to add those 2 tests to the error handling