aiomoto

[dazza-codes]

Description of Change

Fix #583
Fix #665
Fix #753
Fix #755
Fix getmoto/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]

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

[codecov]

Codecov Report

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

@@             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%)	⬇️
aiobotocore/_endpoint_helpers.py	39.13% <0.00%> (-60.87%)	⬇️
aiobotocore/parsers.py	6.25% <0.00%> (-46.88%)	⬇️
aiobotocore/eventstream.py	64.70% <0.00%> (-35.30%)	⬇️
aiobotocore/response.py	66.00% <0.00%> (-34.00%)	⬇️
aiobotocore/args.py	67.74% <0.00%> (-29.04%)	⬇️
... 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.

[dazza-codes]

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.

[thehesiod]

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

[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]

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]

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

[terricain]

This looks great 😄 Might have to loot some of it for aioboto3 once its merged 😉

[lgtm-com]

This pull request introduces 1 alert when merging da1c4d6 into fcf7cd4 - view on LGTM.com

new alerts:

• 1 for Binding a socket to all network interfaces

[dazza-codes]

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

[lgtm-com]

This pull request introduces 2 alerts when merging 1c24619 into 904332b - 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

[dazza-codes]

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.

[chesstrian]

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.

[dazza-codes]

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.

[chesstrian]

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

[thehesiod]

should use a random port or else you can't run tests in parallel

[dazza-codes]

Pushed a new commit to try to use a free port, although I'm not sure the patch will be used in moto.

[thehesiod]

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

[dazza-codes]

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.

[thehesiod]

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

[rajeshn96]

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?

[github-actions]

This pull request has been marked as stale because it has been inactive for more than 60 days. Please update this pull request or it will be automatically closed in 7 days.

[thehesiod]

btw new version of moto does not require an endpoint per service, all services are serviced by the same endpoint so will not be needed. I started working on that here: master...thehesiod/aio-checksum-streaming-moto-bump bunch of tests to fix