-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
TST: Fix failing remote tests #7266
Conversation
Hi there @pllim 👋 - thanks for the pull request! I'm just a friendly 🤖 that checks for issues related to the changelog and making sure that this pull request is milestoned and labeled correctly. This is mainly intended for the maintainers, so if you are not a maintainer you can ignore this, and a maintainer will let you know if any action is required on your part 😃. Everything looks good from my point of view! 👍 If there are any issues with this message, please report them here. |
Remote data/numpy dev job is green -- https://travis-ci.org/astropy/astropy/jobs/350942991
The |
assert quantity_allclose(loc.lon, -74.0059*u.degree) | ||
assert np.allclose(loc.height.value, 0.) | ||
|
||
# Put this one here as buffer to get around Google map API limit per sec. |
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.
Maybe write in the comment that this raises NameResolveError
no matter whether google does or does not impose its limit.
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.
@mhvk , I added clarification. If this looks good to you, please approve so it can be merged. 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.
Looks great; I hope this will solve the false positives. Only a request to clarify the comment...
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.
Looks A-OK to me. Will merge.
Hopefully the -dev + remove runs will now continue to pass for a while, so we can get some rest! |
@mhvk , I am not so sure... See #7267 (comment) |
TST: Fix failing remote tests
I wish we had backported this to LTS. Given that this doesn't have a changelog, now I'm now backporting it to 2.0.6 retrospectively. |
TST: Fix failing remote tests
Sorry for the inconvenience, @bsipocz ! |
No worries, it seemed to be a good idea at the time, but since then the remote test build started to collect all kind of failures, so retrospect it's better to backport all possible fixes there, too. |
Fix #7264
If this works (i.e., make the remote data job green), I think we should merge this sooner than later. When it fails chronically, we tend to ignore it and then get caught by surprise by Numpy dev stuff (since that job runs remote data tests with Numpy dev).