Skip to content
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 for ListenIP interface/IP scoring #555

Merged
merged 2 commits into from Jan 6, 2017
Merged

Conversation

prashantv
Copy link
Contributor

Some prefactoring for #554

bestScore := -1
var bestIP net.IP
// Select the highest scoring IP as the best IP.
for _, iface := range interfaces {

Copy link

Choose a reason for hiding this comment

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

nit: remove newline

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -0,0 +1,80 @@
// Copyright (c) 2015 Uber Technologies, Inc.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: year

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Keeping it 2015 to keep it consistent with the rest of the code, apparently the "year of first publication" is what we want:
http://www.contentious.com/2007/01/07/copyright-notice-is-the-year-really-necessary/

{
msg: "non-local up ipv4 IPAddr address",
iface: net.Interface{Flags: net.FlagUp},
addr: &net.IPNet{IP: testIP},
Copy link
Contributor

Choose a reason for hiding this comment

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

&net.IPAddr

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch, thanks

{
msg: "non-local down ipv6 address",
iface: net.Interface{},
addr: &net.IPAddr{IP: net.ParseIP("2001:db8:a0b:12f0::1")},
Copy link
Contributor

Choose a reason for hiding this comment

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

Declare this IP at the top as well (dedup L65).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor Author

@prashantv prashantv left a comment

Choose a reason for hiding this comment

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

Addressed feedback

@@ -0,0 +1,80 @@
// Copyright (c) 2015 Uber Technologies, Inc.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Keeping it 2015 to keep it consistent with the rest of the code, apparently the "year of first publication" is what we want:
http://www.contentious.com/2007/01/07/copyright-notice-is-the-year-really-necessary/

{
msg: "non-local up ipv4 IPAddr address",
iface: net.Interface{Flags: net.FlagUp},
addr: &net.IPNet{IP: testIP},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch, thanks

{
msg: "non-local down ipv6 address",
iface: net.Interface{},
addr: &net.IPAddr{IP: net.ParseIP("2001:db8:a0b:12f0::1")},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

bestScore := -1
var bestIP net.IP
// Select the highest scoring IP as the best IP.
for _, iface := range interfaces {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@prashantv prashantv merged commit 34b56f3 into dev Jan 6, 2017
@prashantv prashantv deleted the add_listenip_tests branch January 6, 2017 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants