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 bad host test #3917

Merged
merged 4 commits into from
Nov 5, 2020
Merged

Fix bad host test #3917

merged 4 commits into from
Nov 5, 2020

Conversation

jhaiduce
Copy link
Contributor

@jhaiduce jhaiduce commented Nov 4, 2020

Changed the test test_get_fqdn_by_host_on_bad_host to check the exception thrown by socket.gethostbyname_ex using a regex to account for variations in error message returned in different environments.

Also changed the test to use a context manager to test whether an exception is raised. This should prevent the test from incorrectly passing if no exception is thrown.

These changes close #3916.

Requirements check-list

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Appropriate tests are included (unit and/or functional).
  • Appropriate change log entry included.
  • No documentation update required.
  • No dependency changes.

…environments where a different error code and message are returned (this can happen for instance when a VPN is in use). Also rewrote test to use a context manager (this will prevent the test from incorrectly passing if no exception is thrown)
@jhaiduce jhaiduce marked this pull request as ready for review November 4, 2020 16:48
Copy link
Member

@oliver-sanders oliver-sanders left a comment

Choose a reason for hiding this comment

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

Thanks

Comment on lines +98 to +99
[#3917](https://github.com/cylc/cylc-flow/pull/3917) - Fix a bug that caused one of the hostname resolution tests to fail in certain environments.

Copy link
Member

Choose a reason for hiding this comment

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

BTW: We try to keep the changelog focused on user-facing changes (especially as there is going to be a very long list of changes for Cylc8), no probs though.

@oliver-sanders oliver-sanders added this to the cylc-8.0a3 milestone Nov 5, 2020
@oliver-sanders oliver-sanders merged commit 77bd65b into cylc:master Nov 5, 2020
@oliver-sanders oliver-sanders mentioned this pull request Jan 7, 2021
7 tasks
@hjoliver hjoliver modified the milestones: cylc-8.0a3, cylc-8.0b0 Feb 25, 2021
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.

test_get_fqdn_by_host_on_bad_host fails when a VPN is in use
4 participants