skip tests that fail on travis. #303
Conversation
graingert
changed the title
|
much better to actually fix the tests :> skipping things can easily hide errors. |
Some I think are fixable. But there's one that's failing because the CSV data returned can't be parsed properly by |
|
Sure, but better to have travis pass so PRs can be merged
…On 8 May 2017 00:50, "Jeff Reback" ***@***.***> wrote:
much better to actually fix the tests :> skipping things can easily hide
errors.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#303 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAZQTDPQ4gGpfT4NO642l2TCa9v3vDa-ks5r3liogaJpZM4NOOrf>
.
|
But these tests aren't meant to be that controlled. They're testing to make sure that the data from these sources is being returned as expected. Thus, we do want to make sure our code remains compatible with their API at all times. |
That's a double-edged sword there (skipping at will might actually be hiding errors in your implementation). I agree with @jreback that it is better to figure out why tests are failing and patch if necessary (I imagine you and the maintainers can tell whether your changes are causing new failures). As I said before, some of them are fixable, so I would patching as many as you can and skipping any that are out of our control. |
|
i think fixing these are in everyone's interesting. sure some of these should be more smoke tests, but I think testing for valid data for specific fixes dates is really good. if the tests ever DO fail, then you know something has changed. |
|
If you want to test that the api hasn't changed you should have a set of
separate tests that are run with Travis cron. All regular CI tests should
pass without network access
…On 8 May 2017 01:01, "Jeff Reback" ***@***.***> wrote:
@graingert <https://github.com/graingert>
i think fixing these are in everyone's interesting. sure some of these
should be more smoke tests, but I think testing for valid data for specific
fixes dates is really good. if the tests ever DO fail, then you know
something has changed.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#303 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAZQTP33ai2h1YcztSsSGl7vxIQD0AeKks5r3ltigaJpZM4NOOrf>
.
|
Except that if the repository has a good amount of functionality that depends on interfacing with other data sources, like this one. Ultimately, you can debate what the test should be like or organized all you want, but something tells me this PR isn't getting merged as is unless some of those tests are patched or you can provide convincing reasons as to why those tests are not fixable. |
|
You can have tests that check 3rd party endpoints, but they should just
check the real response matches the fixtures
…On 8 May 2017 01:16, "gfyoung" ***@***.***> wrote:
All regular CI tests should pass without network access
Except that if the repository has a lot of functionality that depends on
interfacing with other data sources, like this one.
Ultimately, you can debate what the test should be like or organized, but
something tells me this PR isn't getting merged as is unless some of those
tests are patched or you can provide convincing reasons as to why those
tests are not fixable.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#303 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAZQTABHYHaiVDUamwGce4icsAzTDk73ks5r3l7cgaJpZM4NOOrf>
.
|
What you seem to be pushing for is that there should be a separation between testing functionality and interfacing with 3rd party data sources. Sure, but that doesn't the change fact that there needs to be tests that integrate everything together, which is what we have currently. |
Patches all failing tests due to actual test failures and not incompatibilities with newer versions of pandas. Closes gh-303.
You should strongly consider using a tool like vcrpy, to remove the dependency on third party APIs in your tests.