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

TST/CI: Use moto to mock S3 calls #17325

Closed
TomAugspurger opened this issue Aug 24, 2017 · 9 comments · Fixed by #17388
Closed

TST/CI: Use moto to mock S3 calls #17325

TomAugspurger opened this issue Aug 24, 2017 · 9 comments · Fixed by #17388
Labels
CI Continuous Integration Testing pandas testing functions or related to the test suite
Milestone

Comments

@TomAugspurger
Copy link
Contributor

https://github.com/spulec/moto

I think most of our tests hitting s3 can be mocked now that we're using s3fs and boto3. We don't have any code doing network requests, auth, etc. As an example:

import pytest
import pandas as pd
from moto import mock_s3
import boto3

f = open("pandas/tests/io/data/tips.csv", 'rb')

@mock_s3
def test_foo():
    conn = boto3.resource("s3", region_name="us-east-1")
    conn.create_bucket(Bucket="pandas-test")

    conn.Bucket("pandas-test").put_object(Key="not-a-real-key.csv", Body=f)
    result = pd.read_csv("s3://pandas-test/not-a-real-key.csv")
    assert isinstance(result, pd.DataFrame)
    with pytest.raises(Exception):
        result = pd.read_csv("s3://pandas-test/for-real-not-a-real-key.csv")

It'd probably be good to have a few integration tests that actually hit S3, but I think most can be replaced with mocked calls.

@TomAugspurger TomAugspurger added CI Continuous Integration Difficulty Novice Testing pandas testing functions or related to the test suite labels Aug 24, 2017
@kirkhansen
Copy link
Contributor

kirkhansen commented Aug 25, 2017

I was planning on taking a look at this for my first attempt to contribute to pandas, but am getting lost when trying to add the 'moto' dependency. It seems wrong to stick it in the setup.py install_requires as installation doesn't actually require it, only tests. I see some places in the ci dir and some other CI tool config files where you're installing things like pytest, but can't seem to locate the one that would install test deps with a simple python setup.py develop.
Is there a doc somewhere that outlines all the places that dependencies need to be added?

I'm typically used to seeing a tests_requires key in setup.py or a requirements-dev.txt or something similar to install development only dependencies.

@TomAugspurger
Copy link
Contributor Author

I'm typically used to seeing a tests_requires key in setup.py or a requirements-dev.txt or something similar to install development only dependencies.

We have an issue somewhere about cleaning this up, but were waiting on conda-4 which will have compatible requirements.txt syntax with pip (that's coming out soon I think).

For now, you'll add it to some of the ci/requirements-*.run files. Probably anywhere that currently has s3fs.

  • ci/requirements-2.7.run
  • ci/requirements-2.7_WIN.run
  • ci/requirements-2.7_SLOW.run
  • ci/requirements-3.5.run
  • ci/requirements-3.5_OSX.run
  • ci/requirements-3.6.run
  • ci/requirements-3.6_LOCALE.run
  • ci/requirements-3.6_LOCALE_RUN.run

As far as the tests go, I would probably mock out all the tests currently hitting S3 (mostly in pandas/tests/io/parser/test_network.py, some in test_excel and some in test_s3). And then maybe add back a couple that aren't mocked.

Also, hi 😄

@kirkhansen
Copy link
Contributor

Thanks for the welcome! I am working on this when I get some spare time. All the s3 things I found are mocked out. I'm having the most trouble getting moto installed in all the build envs as it doesn't appear to be in conda, aka, no .run files for me. I've added them to most of the pip files, and make a new pip file for a couple builds, but am still running into annoyances. Just an FYI that I am plugging away at it :)

Do you happen to know if the Windows instance is able to pip install packages? I've marked a couple tests to skip if boto/moto aren't there for that box.

@TomAugspurger
Copy link
Contributor Author

I think moto is on conda-forge here, and we have conda-forge in the channels on the CI servers, so that should be found.

Not sure if you are, but you shouldn't have to build all those envs locally. You can just put up a pull request and they'll run on the various services.

@jreback
Copy link
Contributor

jreback commented Aug 30, 2017

you can simply add to the ci/*.pip as needed
this why it exists

@kirkhansen
Copy link
Contributor

Not sure if you are, but you shouldn't have to build all those envs locally. You can just put up a pull request and they'll run on the various services.

I followed the docs to setup the CI builds with my own fork. The builds and tests I'm referring to are running:

I don't know if everyone has permissions to see those, but most of the failed builds there are due to conda failing to find moto

I think moto is on conda-forge here, and we have conda-forge in the channels on the CI servers, so that should be found.

Hmm, that's interesting. The first builds that ran, I had moto added to all the .run files, and I got errors from conda saying "moto" couldn't be found. Perhaps I'm missing something from my own CI builds with these tools? I've never used these, but it seems everything should be configured via the same ini files, so I'm not sure how that would happen... Maybe I'll just open a PR with what I have tonight, and see how things run.

you can simply add to the ci/*.pip as needed
this why it exists

I did add moto to the existing .pip files, and I created a new one for a linux build (NUMPY-dev or something) that was failing in travis. My question was specifically for the appveyor windows build that fails when it can't find moto for the conda install and there are no existing .pip files for the windows build making me think pip probably isn't installed, thus the question; sorry for the confusion.

@TomAugspurger
Copy link
Contributor Author

Weird, they only uploaded files for a few platforms https://anaconda.org/conda-forge/moto/files

The windows machines should have pip installed, so you could add a file. Otherwise, conda-forge/moto-feedstock#2 is open and close to being ready I think, and should put packages up for all our platforms. We'll see if that gets merged today.

@TomAugspurger
Copy link
Contributor Author

TomAugspurger commented Aug 31, 2017 via email

@kirkhansen
Copy link
Contributor

kirkhansen commented Sep 1, 2017

FYI: the conda-forge moto PR was merged, and the files are starting to tricking into https://anaconda.org/conda-forge/moto/files (no Mac version yet, but that's OK for CI). You may be able to remove some of the pip changes and just use the regular .run files

Looks like most of the linux builds still fail
https://travis-ci.org/kirkhansen/pandas/builds/270657694

@jreback jreback added this to the 0.21.0 milestone Sep 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants