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

fix: do not require networking during tests #10321

Merged
merged 1 commit into from
Dec 21, 2021

Conversation

powersj
Copy link
Contributor

@powersj powersj commented Dec 21, 2021

There are a couple of tests that get run that reach out to external
network devices. Unit tests should not depend on functional networking
or be reaching out to random servers. The DNS server is somewhat
understandable, but the SNMP server is an unknown system that is not
owned by the development team.

Fixes: #9894

There are a couple of tests that get run which reach out to external
network devices. Unit tests should not depend on functional networking
or be reaching out to random servers. The DNS server is somewhat
understandable, but the SNMP server is an unknown system that is not
owned by the development team.

Fixes: influxdata#9894
@telegraf-tiger telegraf-tiger bot added the fix pr to fix corresponding bug label Dec 21, 2021
Copy link
Contributor

@srebhan srebhan 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 to me. Thanks @powersj for tackling those.

@srebhan srebhan self-assigned this Dec 21, 2021
@srebhan srebhan added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Dec 21, 2021
@telegraf-tiger
Copy link
Contributor

@powersj powersj merged commit 6a1442f into influxdata:master Dec 21, 2021
@Hipska
Copy link
Contributor

Hipska commented Dec 22, 2021

I don’t think those snmp checks make an actual connection?

powersj added a commit that referenced this pull request Jan 5, 2022
@Hipska
Copy link
Contributor

Hipska commented Jan 10, 2022

@powersj could you elaborate on skipping the snmp plugin tests?

@powersj
Copy link
Contributor Author

powersj commented Jan 10, 2022

@powersj could you elaborate on skipping the snmp plugin tests?

You can try this out by turning off networking on your system and run those tests. You will see that they attempt to make a connection to 1.2.3.4.

@Hipska
Copy link
Contributor

Hipska commented Jan 10, 2022

And the tests fail in that situation?

@powersj
Copy link
Contributor Author

powersj commented Jan 10, 2022

Yes, the tests fail when the user has no networking.

@Hipska
Copy link
Contributor

Hipska commented Jan 10, 2022

Okay, fine then! 👍

powersj added a commit to powersj/telegraf that referenced this pull request Jan 21, 2022
@powersj powersj deleted the fix/local-test-only branch January 23, 2022 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix pr to fix corresponding bug ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tests require network connectivity
4 participants