aiobotocore icon indicating copy to clipboard operation
aiobotocore copied to clipboard

aiomoto

Open dazza-codes opened this issue 4 years ago • 19 comments

Description of Change

Fix #583 Fix #665 Fix #753 Fix #755 Fix spulec/moto#2706

This most likely replaces #759 and that PR should not be merged.

Notes

  • will not work in py3.5 and that's intentional
  • no idea yet about how to integrate this into the rest of aiobotocore tests, so this PR simply uses nested test directories and namespaced fixtures to try to avoid conflicts, but they might need to be resolved (either in this PR or something following this).
  • the MotoService does not provide a clean backend across tests, by default, so the server fixtures in this PR add and use a MotoService.reset option to clear the backends

dazza-codes avatar Feb 19 '20 17:02 dazza-codes

Pushed up a revision to add a MotoService.reset option to clear moto.backend.BACKENDS for the service required. It seems to work, in the context of other non-aio-moto clients that need to create fixtures, like an s3-bucket.


$ pytest -v tests/aws/
============ test session starts ===================
platform linux -- Python 3.6.7, pytest-5.3.5, py-1.8.1, pluggy-0.13.1 -- /opt/conda/envs/aiobotocore/bin/python
cachedir: .pytest_cache
rootdir: /home/dlweber/src/aiobotocore
plugins: cov-2.8.1, asyncio-0.10.0, forked-1.1.3, xdist-1.31.0
collected 20 items                                                                                                                                                                  

tests/aws/test_aws_fixtures.py::test_aws_credentials PASSED                      [  5%]
tests/aws/test_aws_fixtures.py::test_aws_batch_client PASSED                     [ 10%]
tests/aws/test_aws_fixtures.py::test_aws_ec2_client PASSED                       [ 15%]
tests/aws/test_aws_fixtures.py::test_aws_ecs_client PASSED                       [ 20%]
tests/aws/test_aws_fixtures.py::test_aws_iam_client PASSED                       [ 25%]
tests/aws/test_aws_fixtures.py::test_aws_logs_client PASSED                      [ 30%]
tests/aws/test_aws_fixtures.py::test_aws_s3_client PASSED                        [ 35%]
tests/aws/test_aws_s3.py::test_aws_bucket_access PASSED                          [ 40%]
tests/aws/aio/test_aio_aws_s3.py::test_aio_aws_bucket_access                     [ 45%]
tests/aws/aio/test_aiomoto_clients.py::test_aio_aws_session_credentials PASSED   [ 50%]
tests/aws/aio/test_aiomoto_clients.py::test_aio_aws_batch_client PASSED          [ 55%]
tests/aws/aio/test_aiomoto_clients.py::test_aio_aws_ec2_client PASSED            [ 60%]
tests/aws/aio/test_aiomoto_clients.py::test_aio_aws_ecs_client PASSED            [ 65%]
tests/aws/aio/test_aiomoto_clients.py::test_aio_aws_iam_client PASSED            [ 70%]
tests/aws/aio/test_aiomoto_clients.py::test_aio_aws_logs_client PASSED           [ 75%]
tests/aws/aio/test_aiomoto_clients.py::test_aio_aws_s3_client PASSED             [ 80%]
tests/aws/aio/test_aiomoto_clients.py::test_aio_aws_client PASSED                [ 85%]
tests/aws/aio/test_aiomoto_service.py::test_moto_service PASSED                  [ 90%]
tests/aws/aio/test_aiomoto_service.py::test_moto_batch_service PASSED            [ 95%]
tests/aws/aio/test_aiomoto_service.py::test_moto_s3_service PASSED               [100%]

=========== 20 passed in 10.49s ====

Notes

  • added fixtures and tests that create and access an s3-bucket
  • the moto.backends.BACKENDS["s3"]["global"] model might need closer attention to detail about when and how a model instance is created and retained, but that's probably out of scope here

dazza-codes avatar Feb 20 '20 04:02 dazza-codes

Codecov Report

Merging #766 into master will decrease coverage by 34.11%. The diff coverage is 41.94%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #766       +/-   ##
===========================================
- Coverage   90.83%   56.71%   -34.12%     
===========================================
  Files          12       16        +4     
  Lines         611      878      +267     
===========================================
- Hits          555      498       -57     
- Misses         56      380      +324     
Impacted Files Coverage Δ
aiobotocore/aiomoto/aiomoto_services.py 27.83% <27.83%> (ø)
aiobotocore/aiomoto/aiomoto_fixtures.py 42.85% <42.85%> (ø)
aiobotocore/aiomoto/utils.py 46.66% <46.66%> (ø)
aiobotocore/aiomoto/aws_fixtures.py 63.15% <63.15%> (ø)
aiobotocore/__init__.py 0.00% <0.00%> (-100.00%) :arrow_down:
aiobotocore/_endpoint_helpers.py 39.13% <0.00%> (-60.87%) :arrow_down:
aiobotocore/parsers.py 6.25% <0.00%> (-46.88%) :arrow_down:
aiobotocore/eventstream.py 64.70% <0.00%> (-35.30%) :arrow_down:
aiobotocore/response.py 66.00% <0.00%> (-34.00%) :arrow_down:
aiobotocore/args.py 67.74% <0.00%> (-29.04%) :arrow_down:
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 904332b...178a481. Read the comment docs.

codecov[bot] avatar Feb 20 '20 05:02 codecov[bot]

I can't reproduce the travis test failures locally, it seems to be some problem with the virtualenv not getting all the project packages installed or found for the tests. Unfortunately, it works for me.

dazza-codes avatar Feb 20 '20 06:02 dazza-codes

wow awesome, will take a look if someone doesn't get to this before me

thehesiod avatar Feb 20 '20 08:02 thehesiod

btw the idea for the tests, in regard to buckets is that each test gets a unique bucket, see https://github.com/aio-libs/aiobotocore/blob/master/tests/conftest.py#L30

thehesiod avatar Feb 20 '20 09:02 thehesiod

~~It's been awhile but I think unfortunately moto shares global state across its services. Which is a good but also bad thing. Good because it allows cross service linkages (ex SNS messages triggering lambdas), bad because it means you can't reliably run multiple instances of the same service in the same process. Really it should have a better way of doing this but c'est la vie. So I think we need to re-structure the moto service fixture to ensure that each function scope gets a service helper process, and then each service for the function scope runs in said helper process. This will require a bit of infra around the MotoService class. This will ensure separation of services across tests and allow for cross service linkages for each test.~~

thehesiod avatar Feb 20 '20 09:02 thehesiod

Actually ignore above rec for separate process per test. pytest-xdist already separates each test into a separate process

thehesiod avatar Feb 20 '20 15:02 thehesiod

This looks great :smile: Might have to loot some of it for aioboto3 once its merged :wink:

terricain avatar Feb 20 '20 15:02 terricain

This pull request introduces 1 alert when merging da1c4d68a6240f0650f9654c29ac882eb7f1af80 into fcf7cd49dc9a72cf73267924903fb081152e3f5c - view on LGTM.com

new alerts:

  • 1 for Binding a socket to all network interfaces

lgtm-com[bot] avatar Feb 20 '20 16:02 lgtm-com[bot]

Need to consider options to parametrize the server and client fixtures to address the code-cov problem.

dazza-codes avatar Feb 20 '20 16:02 dazza-codes

This pull request introduces 2 alerts when merging 1c24619f3ec19c2310d44f78437244d44fd7e692 into 904332b2ab0a3cf44c49af13ac04173a71f9e647 - view on LGTM.com

new alerts:

  • 1 for Wrong name for an argument in a call
  • 1 for Binding a socket to all network interfaces

lgtm-com[bot] avatar Feb 23 '20 21:02 lgtm-com[bot]

The PR was rebased on master and seems to pass unit tests OK.

  • make mototest works
  • make coverage does not pass everything (add -m moto?)
  • test coverage doesn't seem to capture everything
    • maybe test coverage can't handle asyncio tracing or it can't test code that is called in pytest fixtures
    • e.g. aiobotocore/aiomoto/aiomoto_services.py should be fully tested already by tests/aiomoto/test_aiomoto_service.py

It's in a state that might be "good enough to go", although there might be additional feature requests that subsequent PRs can address. I have enough code to continue with the "real" work that needs some of this and my focus needs to shift to that. Hope this helps a bit. I did not add any documentation about this, because it could be premature to advertise it.

dazza-codes avatar Feb 23 '20 21:02 dazza-codes

Hi @dazza-codes, do you know if there are any plans to merge this?, I'm able to reproduce the issue AttributeError: 'AWSResponse' object has no attribute 'raw_headers' testing some DynamoDB operations.

Thanks in advance.

chesstrian avatar Apr 03 '20 17:04 chesstrian

My focus on this has moved to https://gitlab.com/dazza-codes/aio-aws and I've lost track of where this PR is at in the context of this project. I doubt it's in the right form exactly to be merged for this project. I don't know if I have time and/or enough feedback to get it into the right shape for this repo. I understand there are some other branches with various refactoring that might conflict with this, so it's probably not worth the time right now to resolve anything until there's a new release. Maybe the project maintainers can just cherrypick patches from this into the main line. I know it took a lot of time and reading to understand the problem and put together this test suite, so let's see what happens as the project refactoring evolves. At least this PR is visible as one possible solution to the test problems.

dazza-codes avatar Apr 04 '20 17:04 dazza-codes

Thanks @dazza-codes, I forked and merged your PR and it didn't work for DynamoDB, need to change the approach to test.

chesstrian avatar Apr 05 '20 20:04 chesstrian

sorry for taking so long to review this, been super busy

thehesiod avatar Jul 15 '20 17:07 thehesiod

I haven't been able to keep up with this and I'm a bit blocked on it since the move to Pipfile (I've been using poetry and avoiding anything pipenv); I don't recall the details of when/why I switched from pipenv to poetry, but I vaguely recall a bunch of issues that totally wrecked my python setups. So far, despite a few minor hiccups with poetry, it's been a good move.

dazza-codes avatar Jul 17 '20 19:07 dazza-codes

nice, we were on pipenv, then got frustrated due to slowness/inability to resolve certain scenarios so moved to poetry, then found another blocking bug on it so switched back :). I initially tried the new pipenv release but it was completely busted so waiting a bit more to try the latest pipenv again

thehesiod avatar Jul 17 '20 22:07 thehesiod

hello, @dazza-codes I too am reproducing this error 'AWSResponse' object has no attribute 'raw_headers' when testing with moto. I tried using the patched code solution suggested in https://github.com/aio-libs/aiobotocore/issues/755#issuecomment-586117397 but it doesn't seem to affect the issue. Maybe I missed it in this thread, but is there a better way to deal with this error?

rajeshn96 avatar Sep 18 '20 13:09 rajeshn96